Skip to content

fix(rigor): make --changed-only dir filtering work outside POSIX - #473

Open
Tyagiquamar wants to merge 1 commit into
understudylabs:mainfrom
Tyagiquamar:fix/rigor-ci-changed-dirs-windows
Open

fix(rigor): make --changed-only dir filtering work outside POSIX#473
Tyagiquamar wants to merge 1 commit into
understudylabs:mainfrom
Tyagiquamar:fix/rigor-ci-changed-dirs-windows

Conversation

@Tyagiquamar

Copy link
Copy Markdown

Problem

benchmarks rigor --ci --changed-only silently degenerates on Windows. Two independent POSIX-only assumptions in filterChangedBenchmarkDirs() (src/rigor-report.ts):

  1. The changed-file prefix check compared against `${resolve(dir)}/` — a hardcoded forward slash appended to a native-separated absolute path. On Windows resolve() yields backslashes, so C:\repo\bench-a\file never starts with C:\repo\bench-a/, every genuinely touched benchmark dir is filtered out, and CI checks nothing (the opposite failure mode of the documented fail-open fallback, and much quieter).
  2. The merge-base lookup ran through an explicit POSIX shell (shell: "/bin/sh") to get 2>/dev/null + || chaining. /bin/sh doesn't exist on Windows, so the whole lookup throws and the function falls back to "everything changed".

Found by running the test suite on Windows: both tests/rigor-ci.test.mjs cases — "keeps exactly the dirs touched since the base ref" (actual [], expected [...bench-a]) and "resolves git's repo-root-relative paths correctly from a subdirectory cwd" — fail at these lines.

Fix

  • Compare file === resolve(dir) or file.startsWith(resolve(dir) + sep) so the boundary respects the platform separator.
  • Split the fallback chain into two execSync calls (git merge-base HEAD origin/main, falling back to git rev-parse HEAD~1 on failure) with no explicit shell. Same resolution order as before; Node's default shell handles both commands on POSIX and Windows.

POSIX behavior is unchanged: identical base-ref selection, identical dir ordering, same fail-open catch.

Testing

On Windows 11, Node 24:

node --test tests/rigor-ci.test.mjs tests/rigor-report.test.mjs

before: exit 1 — "keeps exactly the dirs touched since the base ref" and
        "resolves git's repo-root-relative paths correctly from a subdirectory cwd" fail
after:  exit 0 — tests 23, pass 23, fail 0

npm run typecheck → exit 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant