tests/opa: normalize path separators in folder filter on Windows#742
Merged
anakrish merged 1 commit intoJun 5, 2026
Merged
Conversation
`run_opa_tests` builds `path_dir_str` from `path.strip_prefix(...).to_string_lossy()`,
which on Windows yields strings with backslash separators (e.g.
`v0\aggregates`). The folder filter then does an exact-string
comparison against the CLI arguments:
let run_test = folders.is_empty()
|| folders.iter().any(|f| &path_dir_str == f);
CLI arguments use forward slashes (`v0/aggregates`), so on Windows
the comparison never matches, no tests are selected, and the function
bails with `"no matching tests found"`. This blocks the
`cargo xtask pre-push` hook for any Windows contributor.
Normalize `path_dir_str` to use forward slashes at construction
time. Reproduces before the fix as `cargo test ... --test opa --
v1/aggregates` exiting 1 with `no matching tests found`; after the
fix the same command runs 72 cases and the full hook command runs
2861 / 0 across 188 folders.
The duplicate platform check at the `is_rego_v0_test` site
(`path_dir_str.starts_with("v0/") || path_dir.starts_with("v0\\")`)
is left intact to keep the change minimal — the backslash branch
becomes redundant but is harmless.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes Windows-only failures in the OPA conformance test runner by normalizing directory paths used for folder filtering so they match the forward-slash format passed via CLI (and the tests/opa.passing list), preventing the “no matching tests found” early exit.
Changes:
- Normalize
path_dir_strto use/separators when deriving folder names inrun_opa_tests. - Add an explanatory comment documenting the Windows path-separator mismatch and why normalization is needed.
anakrish
approved these changes
Jun 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Windows,
cargo xtask pre-pushfails immediately at the OPA testsuite stage withError: no matching tests found., even though the testsuite is present and every other check passes. The cause is a path-separator mismatch in the folder filter intests/opa.rs.Root cause
run_opa_testsbuildspath_dir_strfromPath::to_string_lossy(), which on Windows yields backslash separators (e.g.v0\aggregates). The folder filter then does an exact-string comparison against the CLI arguments:CLI arguments — including the ones the
cargo xtask pre-pushhook passes — use forward slashes (v0/aggregates). On Windows the equality check never matches, no folders are selected,npass == 0 && nfail == 0, and the function bails. The neighboringis_rego_v0_testcheck already accounts for the separator difference (starts_with("v0/") || path_dir.starts_with("v0\\")), but the filter comparison was missed.Reproduction
On Windows, against
mainat11940dd:Output:
Exit code
1. The same failure blockscargo xtask pre-pushfor any change on this platform.Fix
Normalize
path_dir_strto forward slashes at construction time:Single line of logic. No behavior change on Unix-like platforms (no backslashes to replace).
Validation
After the fix, on the same Windows machine:
cargo test ... --test opa -- v1/aggregatesno matching tests foundcargo test ... --test opa -- <all 188 v0/* and v1/* folders>(the pre-push hook command)no matching tests foundcargo xtask pre-commitNotes
path_dir.starts_with("v0\\")becomes redundant after this change but is left intact to keep the diff minimal — happy to remove it in a follow-up if preferred.