Skip to content

Commit 2d44be3

Browse files
branchseerclaude
andauthored
chore(test): mark integration tests as ignored to decouple from Node.js setup (#341)
## Motivation Two goals, both aimed at making the test suite behave more like standard libtest and be easier for humans and AI agents to reason about: 1. **Default `cargo test` should pass with only the Rust toolchain.** Tests that need Node.js or `pnpm install` are marked `#[ignore]` (or `ignore = true` in an `[[e2e]]` TOML block). AI agents and new contributors can run and iterate on tests without the extra setup; CI runs the Rust-only tests first, then sets up Node/pnpm and runs the ignored set. 2. **Trial names should look like standard libtest identifiers.** Every fixture folder name and `[[e2e]] / [[plan]]` case name is now `[A-Za-z0-9_]` only, so `cargo test task_select::interactive_select_task` works without quoting, snapshot filenames carry no whitespace, and the mapping between a trial name and its on-disk fixture is obvious. A build-time assertion rejects any future name that breaks the rule. ## Key Changes - **One libtest-mimic trial per e2e case** (was one per fixture). `run_case_inner` → `run_case` takes a single `E2e`, trials are named `{fixture}::{case}`, and per-case staging dirs (`{fixture}_case_{index}`) keep parallel trials isolated. - **`snapshots.toml` loaded once per fixture** at trial-list build time via a new `load_snapshots_file` helper. The per-case runner no longer re-reads the file. - **`ignore = true` under `[[e2e]]`** maps directly onto `libtest_mimic::Trial::with_ignored_flag(...)`. - **Node.js / pnpm-dependent tests marked `#[ignore]`**: - `crates/fspy/tests/node_fs.rs`: all 8 tests (need `node`) - `crates/fspy/tests/oxlint.rs`: all 3 tests (need `oxlint` from `packages/tools`) - `crates/vite_task_bin/tests/e2e_snapshots/fixtures/signal_exit/snapshots.toml`: the single case that runs `node -e ...` - **Fixture folder and case name normalization**: 35 e2e folders + 45 plan folders renamed (`task-select` → `task_select`, etc.); 149 e2e + 117 plan case names normalized (any non-`[A-Za-z0-9_]` → `_`); all `.snap` / `.jsonc` files renamed in lock-step. Plan harness hardcoded names updated: `"task graph"` → `"task_graph"`, `"task graph load error"` → `"task_graph_load_error"`, `"query - {name}"` → `"query_{name}"`. - **Build-time guard**: both e2e and plan harnesses now call `assert_identifier_like(...)` on fixture folder names and case names during trial construction, so non-conforming entries fail loudly with a clear message. - **CI workflow reorder** (both the matrix `test` job and `test-musl`): `cargo test --no-run` → `cargo test` → install Node.js + pnpm → `cargo test -- --ignored`. The Rust-only run happens before Node/pnpm are on PATH, enforcing the decoupling on every PR. - **Docs**: `CONTRIBUTING.md` and `CLAUDE.md` describe the Rust-only default and how to run the ignored subset (`cargo test -- --include-ignored` or `--ignored`) after `pnpm install` at the workspace root (pnpm workspaces also install `packages/tools`). ## Implementation Details - The `ignored` flag is applied via `libtest_mimic::Trial::with_ignored_flag(bool)` — no reason string, matching libtest-mimic's API. - Platform filtering (`platform = "unix"` / `"windows"`) is now applied during trial generation, so unmatched cases don't appear in the trial list at all. - A handful of gitignored fixture files (`dist/output.js`, `node_modules/.bin/vt{,.cmd}`) needed `git add -f` at their new paths to survive the folder rename. https://claude.ai/code/session_01WHUFjqS6EbcoXp4UWHN1rj --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 954cd3b commit 2d44be3

706 files changed

Lines changed: 420 additions & 346 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/ci.yml

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,15 @@ jobs:
123123
- uses: mlugg/setup-zig@d1434d08867e3ee9daa34448df10607b98908d29 # v2
124124
if: ${{ matrix.target == 'x86_64-unknown-linux-gnu' }}
125125

126+
- name: Build tests
127+
run: ${{ matrix.cargo_cmd }} test --no-run --target ${{ matrix.build_target }}
128+
129+
# Default `cargo test` runs only tests that need nothing beyond the
130+
# Rust toolchain; this step verifies that contract before Node.js
131+
# and pnpm enter the picture.
132+
- name: Run tests
133+
run: ${{ matrix.cargo_cmd }} test --target ${{ matrix.build_target }}
134+
126135
# For x86_64-apple-darwin on arm64 runner, install x64 node so fspy preload dylib
127136
# (compiled for x86_64) can be injected into node processes running under Rosetta.
128137
# oxc-project/setup-node doesn't support the architecture input, so use
@@ -143,11 +152,8 @@ jobs:
143152
# Must run after setup-node so correct native binaries are installed
144153
- run: pnpm install
145154

146-
- name: Build tests
147-
run: ${{ matrix.cargo_cmd }} test --no-run --target ${{ matrix.build_target }}
148-
149-
- name: Run tests
150-
run: ${{ matrix.cargo_cmd }} test --target ${{ matrix.build_target }}
155+
- name: Run ignored tests
156+
run: ${{ matrix.cargo_cmd }} test --target ${{ matrix.build_target }} -- --ignored
151157

152158
test-musl:
153159
needs: detect-changes
@@ -186,17 +192,23 @@ jobs:
186192
- name: Install Rust toolchain
187193
run: rustup show
188194

189-
- name: Install pnpm and Node tools
190-
run: |
191-
corepack enable
192-
pnpm install
193-
194195
- name: Build tests
195196
run: cargo test --no-run
196197

198+
# Default `cargo test` runs only tests that need nothing beyond the
199+
# Rust toolchain; this step verifies that contract before pnpm
200+
# populates `packages/tools/node_modules`.
197201
- name: Run tests
198202
run: cargo test
199203

204+
- name: Install pnpm and Node tools
205+
run: |
206+
corepack enable
207+
pnpm install
208+
209+
- name: Run ignored tests
210+
run: cargo test -- --ignored
211+
200212
fmt:
201213
name: Format and Check Deps
202214
runs-on: namespace-profile-linux-x64-default

.typos.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ extend-exclude = [
88
"crates/fspy_detours_sys/src/generated_bindings.rs",
99
# Intentional typos for testing fuzzy matching and "did you mean" suggestions
1010
"crates/vite_select/src/fuzzy.rs",
11-
"crates/vite_task_bin/tests/e2e_snapshots/fixtures/task-select",
11+
"crates/vite_task_bin/tests/e2e_snapshots/fixtures/task_select",
1212
]

CLAUDE.md

Lines changed: 1 addition & 1 deletion

CONTRIBUTING.md

Lines changed: 9 additions & 1 deletion

crates/fspy/tests/node_fs.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,27 +23,31 @@ async fn track_node_script(script: &str, args: &[&OsStr]) -> anyhow::Result<Path
2323
}
2424

2525
#[test(tokio::test)]
26+
#[ignore = "requires node"]
2627
async fn read_sync() -> anyhow::Result<()> {
2728
let accesses = track_node_script("try { fs.readFileSync('hello') } catch {}", &[]).await?;
2829
assert_contains(&accesses, current_dir().unwrap().join("hello").as_path(), AccessMode::READ);
2930
Ok(())
3031
}
3132

3233
#[test(tokio::test)]
34+
#[ignore = "requires node"]
3335
async fn exist_sync() -> anyhow::Result<()> {
3436
let accesses = track_node_script("try { fs.existsSync('hello') } catch {}", &[]).await?;
3537
assert_contains(&accesses, current_dir().unwrap().join("hello").as_path(), AccessMode::READ);
3638
Ok(())
3739
}
3840

3941
#[test(tokio::test)]
42+
#[ignore = "requires node"]
4043
async fn stat_sync() -> anyhow::Result<()> {
4144
let accesses = track_node_script("try { fs.statSync('hello') } catch {}", &[]).await?;
4245
assert_contains(&accesses, current_dir().unwrap().join("hello").as_path(), AccessMode::READ);
4346
Ok(())
4447
}
4548

4649
#[test(tokio::test)]
50+
#[ignore = "requires node"]
4751
async fn create_read_stream() -> anyhow::Result<()> {
4852
let accesses = track_node_script(
4953
"try { fs.createReadStream('hello').on('error', () => {}) } catch {}",
@@ -55,6 +59,7 @@ async fn create_read_stream() -> anyhow::Result<()> {
5559
}
5660

5761
#[test(tokio::test)]
62+
#[ignore = "requires node"]
5863
async fn create_write_stream() -> anyhow::Result<()> {
5964
let tmpdir = tempfile::tempdir()?;
6065
let file_path = tmpdir.path().join("hello");
@@ -68,6 +73,7 @@ async fn create_write_stream() -> anyhow::Result<()> {
6873
}
6974

7075
#[test(tokio::test)]
76+
#[ignore = "requires node"]
7177
async fn write_sync() -> anyhow::Result<()> {
7278
let tmpdir = tempfile::tempdir()?;
7379
let file_path = tmpdir.path().join("hello");
@@ -81,13 +87,15 @@ async fn write_sync() -> anyhow::Result<()> {
8187
}
8288

8389
#[test(tokio::test)]
90+
#[ignore = "requires node"]
8491
async fn read_dir_sync() -> anyhow::Result<()> {
8592
let accesses = track_node_script("try { fs.readdirSync('.') } catch {}", &[]).await?;
8693
assert_contains(&accesses, &current_dir().unwrap(), AccessMode::READ_DIR);
8794
Ok(())
8895
}
8996

9097
#[test(tokio::test)]
98+
#[ignore = "requires node"]
9199
async fn subprocess() -> anyhow::Result<()> {
92100
let cmd = if cfg!(windows) {
93101
r"'cmd', ['/c', 'type hello']"

crates/fspy/tests/oxlint.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ async fn track_oxlint(dir: &std::path::Path, args: &[&str]) -> anyhow::Result<Pa
5252
}
5353

5454
#[test(tokio::test)]
55+
#[ignore = "requires `pnpm install` (workspace root)"]
5556
async fn oxlint_reads_js_file() -> anyhow::Result<()> {
5657
let tmpdir = tempfile::tempdir()?;
5758
// on macOS, tmpdir.path() may be a symlink, so we need to canonicalize it
@@ -69,6 +70,7 @@ async fn oxlint_reads_js_file() -> anyhow::Result<()> {
6970
}
7071

7172
#[test(tokio::test)]
73+
#[ignore = "requires `pnpm install` (workspace root)"]
7274
async fn oxlint_reads_directory() -> anyhow::Result<()> {
7375
let tmpdir = tempfile::tempdir()?;
7476

@@ -84,6 +86,7 @@ async fn oxlint_reads_directory() -> anyhow::Result<()> {
8486
}
8587

8688
#[test(tokio::test)]
89+
#[ignore = "requires `pnpm install` (workspace root)"]
8790
async fn oxlint_type_aware() -> anyhow::Result<()> {
8891
let tmpdir = tempfile::tempdir()?;
8992
// on macOS, tmpdir.path() may be a symlink, so we need to canonicalize it

crates/vite_task_bin/tests/e2e_snapshots/fixtures/associate-existing-cache/package.json renamed to crates/vite_task_bin/tests/e2e_snapshots/fixtures/associate_existing_cache/package.json

File renamed without changes.

crates/vite_task_bin/tests/e2e_snapshots/fixtures/associate-existing-cache/snapshots.toml renamed to crates/vite_task_bin/tests/e2e_snapshots/fixtures/associate_existing_cache/snapshots.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Tests that tasks with identical commands share cache
22

33
[[e2e]]
4-
name = "associate existing cache"
4+
name = "associate_existing_cache"
55
steps = [
66
{ argv = [
77
"vt",

crates/vite_task_bin/tests/e2e_snapshots/fixtures/associate-existing-cache/snapshots/associate existing cache.snap renamed to crates/vite_task_bin/tests/e2e_snapshots/fixtures/associate_existing_cache/snapshots/associate_existing_cache.snap

File renamed without changes.

crates/vite_task_bin/tests/e2e_snapshots/fixtures/associate-existing-cache/vite-task.json renamed to crates/vite_task_bin/tests/e2e_snapshots/fixtures/associate_existing_cache/vite-task.json

File renamed without changes.

0 commit comments

Comments
 (0)