From 35b08e7e2edb75d0f25bd3881d2064d54220e516 Mon Sep 17 00:00:00 2001 From: Elijah Zupancic Date: Tue, 2 Jun 2026 07:26:26 -0700 Subject: [PATCH 1/2] fix(cli): make diff keys and a preproc test Windows path-safe Two Windows-only CI failures (run 26799627900), both `/` vs `\` separator issues in the #482 epic work, latent until the push hit Windows CI (local dev is Linux). - metric_diff `path_to_key`: `to_str()` kept the OS separator, so a directory-set's relpath fallback key was `nested\y.json` on Windows and could not pair with a `/`-keyed set. Normalize the separator to `/` so directory-set diff keys are platform-independent, matching the `name`-field branch's `/`-form. - cli_smoke `preproc_resolves_cross_file_include_across_directory`: the test built `helper_h` via `join("sub/helper.h")`, a mixed-separator path on Windows (`...\sub/helper.h`) that never matched the walk-emitted, backslash-separated `indirect_includes` path. Build the path component-by-component so it uses the OS separator. Test-only; the production preproc output was internally consistent. Both changes are no-ops on Linux (verified the two tests plus the metric_diff / cli_smoke / diff_since / check_exclude / check_baseline suites still pass). Baseline refreshed for the one-closure growth in metric_diff.rs. --- .bca-baseline.toml | 6 +++--- big-code-analysis-cli/src/metric_diff.rs | 8 +++++++- big-code-analysis-cli/tests/cli_smoke.rs | 6 +++++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.bca-baseline.toml b/.bca-baseline.toml index 31018348..5e0671ae 100644 --- a/.bca-baseline.toml +++ b/.bca-baseline.toml @@ -625,14 +625,14 @@ path = "big-code-analysis-cli/src/metric_diff.rs" qualified = "" start_line = 1 metric = "halstead.effort" -value = 1579186.894855429 +value = 1578036.5821063574 [[entry]] path = "big-code-analysis-cli/src/metric_diff.rs" qualified = "" start_line = 1 metric = "nargs" -value = 43.0 +value = 44.0 [[entry]] path = "big-code-analysis-cli/src/metric_diff.rs" @@ -646,7 +646,7 @@ path = "big-code-analysis-cli/src/metric_diff.rs" qualified = "" start_line = 1 metric = "nom" -value = 39.0 +value = 40.0 [[entry]] path = "big-code-analysis-cli/src/metric_diff.rs" diff --git a/big-code-analysis-cli/src/metric_diff.rs b/big-code-analysis-cli/src/metric_diff.rs index dd06a5b6..b4576853 100644 --- a/big-code-analysis-cli/src/metric_diff.rs +++ b/big-code-analysis-cli/src/metric_diff.rs @@ -533,9 +533,15 @@ fn read_json(path: &Path) -> Result { /// Convert a path into a stable string key, erroring on non-UTF-8 rather /// than lossily transcoding (identifier paths must round-trip). +/// +/// The OS separator is normalized to `/` so a directory-set's relpath +/// fallback key is platform-independent: a file at `nested/y.json` keys +/// identically whether the walk ran on Unix or Windows (where +/// `to_str()` would otherwise yield `nested\y.json`, breaking pairing +/// against a `/`-keyed set and the documented `bca diff` contract). fn path_to_key(path: &Path) -> Result { path.to_str() - .map(str::to_string) + .map(|s| s.replace(std::path::MAIN_SEPARATOR, "/")) .ok_or_else(|| DiffError::NonUtf8Path { path: path.to_path_buf(), }) diff --git a/big-code-analysis-cli/tests/cli_smoke.rs b/big-code-analysis-cli/tests/cli_smoke.rs index 95c616db..efa114a2 100644 --- a/big-code-analysis-cli/tests/cli_smoke.rs +++ b/big-code-analysis-cli/tests/cli_smoke.rs @@ -230,7 +230,11 @@ fn preproc_resolves_cross_file_include_across_directory() { let dir = TempDir::new().unwrap(); std::fs::create_dir(dir.path().join("sub")).unwrap(); let main_c = dir.path().join("main.c"); - let helper_h = dir.path().join("sub/helper.h"); + // Join components separately so the path uses the OS separator: a + // literal `"sub/helper.h"` would leave a forward slash on Windows + // (mixed `...\sub/helper.h`) that never matches the walk-emitted, + // backslash-separated `indirect_includes` path. + let helper_h = dir.path().join("sub").join("helper.h"); std::fs::write( &main_c, "#include \"helper.h\"\nint main(void){ return HELPER; }\n", From 6d14e3b5184497694070a2412df4a08276c1ee34 Mon Sep 17 00:00:00 2001 From: Elijah Zupancic Date: Tue, 2 Jun 2026 08:15:15 -0700 Subject: [PATCH 2/2] fix(test): scrub diff-scope env vars so PR-event CI has clean stderr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #498's CI failed the `test` job on all three platforms — 10 integration tests asserting clean stderr got: bca: --since/auto-detect via GITHUB_BASE_REF failed (not inside a git checkout); proceeding without diff scope Root cause (pre-existing, unrelated to the Windows path fix): `bca check` auto-detects a diff scope from `GITHUB_BASE_REF` / `BCA_DIFF_BASE` / `GITHUB_EVENT_BEFORE` (diff.rs `auto_detect_base`), but the test harness's `scrub_ci_env` only stripped `GITHUB_STEP_SUMMARY` / `GITHUB_ACTIONS`. On a `pull_request` event GitHub sets `GITHUB_BASE_REF`, so the hermetic-tempdir `bca check` invocations auto-enabled `--since`, failed to resolve `origin/main` (the tempdir is not a git checkout), and warned to stderr. Push events leave `GITHUB_BASE_REF` unset, so this stayed latent until the first PR — the prior push-to-main runs never set it. Scrub the three auto-detect vars in `scrub_ci_env` (all integration tests route through it). Verified test-via-revert: with `GITHUB_BASE_REF=main` the suppression test fails without the scrub and passes with it; the full CLI suite and the whole workspace are green under the simulated PR env. A test that wants a diff scope still sets the var explicitly after construction. --- big-code-analysis-cli/tests/common/mod.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/big-code-analysis-cli/tests/common/mod.rs b/big-code-analysis-cli/tests/common/mod.rs index 1a6cbb43..86c1f532 100644 --- a/big-code-analysis-cli/tests/common/mod.rs +++ b/big-code-analysis-cli/tests/common/mod.rs @@ -37,6 +37,17 @@ pub mod validators; /// bounded by fixed sentinels, the last test wins, replacing every /// earlier block (see #388). /// +/// The diff-scope auto-detection vars (`diff.rs::auto_detect_base`) +/// are scrubbed for the same reason: on a `pull_request` event GitHub +/// sets `GITHUB_BASE_REF` (and a push sets `GITHUB_EVENT_BEFORE`), so a +/// hermetic-tempdir `bca check` would auto-enable a `--since` scope, +/// fail to resolve `origin/` (the tempdir is not a git checkout), +/// and emit a "proceeding without diff scope" warning to stderr — +/// breaking every test that asserts clean stderr. This only surfaces on +/// `pull_request` CI events (push runs leave `GITHUB_BASE_REF` unset), +/// which is why it stayed latent until the first PR. A test that +/// *wants* a diff scope sets the var explicitly after construction. +/// /// Call sites name their builder `cli()` (or `bin()` in /// `big-code-analysis-web`); each delegates to this helper so a /// future new env-leak only needs to be patched once. @@ -44,6 +55,9 @@ pub mod validators; pub fn scrub_ci_env(cmd: &mut Command) -> &mut Command { cmd.env_remove("GITHUB_STEP_SUMMARY") .env_remove("GITHUB_ACTIONS") + .env_remove("GITHUB_BASE_REF") + .env_remove("BCA_DIFF_BASE") + .env_remove("GITHUB_EVENT_BEFORE") } /// Build a `bca` `Command` with CI-side env vars scrubbed. The