Skip to content

tests/opa: normalize path separators in folder filter on Windows#742

Merged
anakrish merged 1 commit into
microsoft:mainfrom
kusha:markbirger/opa-test-windows-path-separator
Jun 5, 2026
Merged

tests/opa: normalize path separators in folder filter on Windows#742
anakrish merged 1 commit into
microsoft:mainfrom
kusha:markbirger/opa-test-windows-path-separator

Conversation

@kusha

@kusha kusha commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

On Windows, cargo xtask pre-push fails immediately at the OPA testsuite stage with Error: 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 in tests/opa.rs.

Root cause

run_opa_tests builds path_dir_str from Path::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:

let path_dir_str = path_dir.to_string_lossy().to_string();
// ...
let run_test = folders.is_empty()
    || folders.iter().any(|f| &path_dir_str == f);

CLI arguments — including the ones the cargo xtask pre-push hook 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 neighboring is_rego_v0_test check already accounts for the separator difference (starts_with("v0/") || path_dir.starts_with("v0\\")), but the filter comparison was missed.

Reproduction

On Windows, against main at 11940dd:

cargo test --features opa-testutil,serde_json/arbitrary_precision,rego-extensions \
  --test opa -- v1/aggregates

Output:

OPA TESTSUITE STATUS
    FOLDER                                    PASS FAIL

Error: no matching tests found.

Exit code 1. The same failure blocks cargo xtask pre-push for any change on this platform.

Fix

Normalize path_dir_str to forward slashes at construction time:

-        let path_dir_str = path_dir.to_string_lossy().to_string();
+        // Normalize to forward slashes so the folder filter at the
+        // `folders.iter().any(|f| &path_dir_str == f)` check below matches
+        // CLI arguments like `v0/aggregates` on Windows, where
+        // `to_string_lossy` yields backslash separators by default.
+        let path_dir_str = path_dir.to_string_lossy().replace('\\', "/");

Single line of logic. No behavior change on Unix-like platforms (no backslashes to replace).

Validation

After the fix, on the same Windows machine:

Command Before After
cargo test ... --test opa -- v1/aggregates exit 1, no matching tests found exit 0, 72 / 0
cargo test ... --test opa -- <all 188 v0/* and v1/* folders> (the pre-push hook command) exit 1, no matching tests found exit 0, TOTAL: 2861 / 0
cargo xtask pre-commit passes passes (unaffected)

Notes

  • The duplicate platform branch at 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.
  • This is a test-only change; no library or RVM code is touched.

`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>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_str to use / separators when deriving folder names in run_opa_tests.
  • Add an explanatory comment documenting the Windows path-separator mismatch and why normalization is needed.

Comment thread tests/opa.rs
@anakrish anakrish merged commit bd90453 into microsoft:main Jun 5, 2026
59 checks passed
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.

3 participants