Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jj_realistic_needs_to_be_more_clever failure is not limited to CI #1575

Open
EliahKagan opened this issue Sep 5, 2024 · 2 comments
Open
Labels
acknowledged an issue is accepted as shortcoming to be fixed help wanted Extra attention is needed

Comments

@EliahKagan
Copy link
Member

Current behavior 😯

The test case jj_realistic_needs_to_be_more_clever was added in 7ef1e88 (#1529). It was known to fail in Windows on CI:

https://github.com/Byron/gitoxide/blob/d69c6175574f34d6df92b4488ed2c9a85df12c89/gix/tests/object/tree/diff.rs#L396-L401

As indicated in the comment there, it was found pass when run locally on some Windows system. However, it fails when I run in on my main Windows 10 (x86-64) development system. So the failure is not limited to CI.

ek@Glub MINGW64 ~/source/repos/gitoxide (main)
$ RUST_BACKTRACE=1 cargo nextest run -p gix jj_realistic_needs_to_be_more_clever
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.66s
------------
 Nextest run ID 845dbb1b-4fc5-470c-a3b8-6daa5649fd54 with nextest profile: default
    Starting 1 test across 4 binaries (262 tests skipped)
        FAIL [   0.166s] gix::gix object::tree::diff::track_rewrites::jj_realistic_needs_to_be_more_clever

--- STDOUT:              gix::gix object::tree::diff::track_rewrites::jj_realistic_needs_to_be_more_clever ---

running 1 test
test object::tree::diff::track_rewrites::jj_realistic_needs_to_be_more_clever ... FAILED

failures:

failures:
    object::tree::diff::track_rewrites::jj_realistic_needs_to_be_more_clever

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 258 filtered out; finished in 0.14s


--- STDERR:              gix::gix object::tree::diff::track_rewrites::jj_realistic_needs_to_be_more_clever ---
thread 'object::tree::diff::track_rewrites::jj_realistic_needs_to_be_more_clever' panicked at gix\tests\object\tree\diff.rs:452:9:
assertion `left == right` failed
  left: {"cli/src/commands/file/print.rs": ("cli/src/commands/cat.rs", 90), "cli/tests/test_file_print_command.rs": ("cli/tests/test_cat_command.rs", 77), "cli/tests/test_file_chmod_command.rs": ("cli/tests/test_chmod_command.rs", 88), "cli/src/commands/file/chmod.rs": ("cli/src/commands/chmod.rs", 90)}
 right: {}
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library\std\src\panicking.rs:665
   1: core::panicking::panic_fmt
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library\core\src\panicking.rs:74
   2: core::panicking::assert_failed_inner
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library\core\src\panicking.rs:410
   3: core::panicking::assert_failed<std::collections::hash::map::HashMap<ref$<bstr::bstr::BStr>,tuple$<ref$<bstr::bstr::BStr>,u32>,std::hash::random::RandomState>,std::collections::hash::map::HashMap<ref$<bstr::bstr::BStr>,tuple$<ref$<bstr::bstr::BStr>,u32>,std
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c\library\core\src\panicking.rs:365
   4: gix::object::tree::diff::track_rewrites::jj_realistic_needs_to_be_more_clever
             at .\tests\object\tree\diff.rs:452
   5: gix::object::tree::diff::track_rewrites::jj_realistic_needs_to_be_more_clever::closure$0
             at .\tests\object\tree\diff.rs:395
   6: core::ops::function::FnOnce::call_once<gix::object::tree::diff::track_rewrites::jj_realistic_needs_to_be_more_clever::closure_env$0,tuple$<> >
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c\library\core\src\ops\function.rs:250
   7: core::ops::function::FnOnce::call_once
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library\core\src\ops\function.rs:250
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

   Canceling due to test failure
------------
     Summary [   0.169s] 1 test run: 0 passed, 1 failed, 262 skipped
        FAIL [   0.166s] gix::gix object::tree::diff::track_rewrites::jj_realistic_needs_to_be_more_clever
error: test run failed

This run is from the current tip of main, but I believe it has always failed on this system, ever since it was introduced in #1529. (I just didn't get around to reporting it until now.)

All other tests pass on this system. Here's a full run.

Expected behavior 🤔

All tests, including jj_realistic_needs_to_be_more_clever, should pass.

However, I am not opening this to advocate that the test also be skipped locally or marked as ignored. Probably the fixture or whatever is causing the problem should be fixed, rather than modifying the test to run in even fewer circumstances, since local failures are less of a problem for development in other areas of the project than CI failures would be.

(I am opening this issue in part to report the problem, which does not seem previously to have been known to occur outside CI, but in part so there is an issue to link to when showing full test runs, to explain the presence of this test failure when it is not related to the main topic of an issue or PR.)

Git behavior

I don't know of anything that directly corresponds to this in Git or its test suite, but it could well be that investigation of the fixture would reveal an area where the behavior of gitoxide undesirably differs from that of Git. The existing code comment suggests that the problem is instead in the fixture script or the way it is run, however.

Steps to reproduce 🕹

I do not know what distinguishes my local Windows system from the Windows virtual machine on which the test was found to pass.

The failure occurs every time I run the test on Windows, either together with other tests:

cargo nextest run --all --no-fail-fast

Or by itself:

cargo nextest run -p gix jj_realistic_needs_to_be_more_clever
@Byron Byron added help wanted Extra attention is needed acknowledged an issue is accepted as shortcoming to be fixed labels Sep 6, 2024
@Byron
Copy link
Member

Byron commented Sep 6, 2024

Thanks a lot for making me aware!

I do not know what distinguishes my local Windows system from the Windows virtual machine on which the test was found to pass.

It could be that I executed the fixture on a shared drive, and not on the local VM drive. And even if it was the local VM drive, it's also just a simulated disk after all - maybe it's related to that.

Maybe as a quick fix, there could be a PR that skips this test on Windows entirely (so it won't be impeding you ), until it's clear what the problem is.
If the fixture truly is the problem, then it should be possible to see what the difference is between what's there and what should be there.
After all, I am quite confident that this doesn't point at a rewrite-tracking related issue on Windows, as this is purely 'in git'.

EliahKagan added a commit to EliahKagan/gitoxide that referenced this issue Sep 6, 2024
This conditionally marks the `jj_realistic_needs_to_be_more_clever`
test as ignored, when the target is Windows. But the test is still
build on Windows, and can be run with:

    cargo nextest run -p gix -- --ignored jj_realistic_needs_to_be_more_clever

This is a workaround for GitoxideLabs#1575.

When run on Windows, the test previously checked if it was running
on CI and skipped its contents. This removes that logic, since the
test no longer runs by default on CI or otherwise.
@EliahKagan
Copy link
Member Author

Maybe as a quick fix, there could be a PR that skips this test on Windows entirely (so it won't be impeding you ), until it's clear what the problem is.

I've opened #1584 for this.

EliahKagan added a commit to EliahKagan/gitoxide that referenced this issue Sep 6, 2024
This conditionally marks the `jj_realistic_needs_to_be_more_clever`
test as ignored, when the target is Windows. But the test is still
built on Windows, and can be run with:

    cargo nextest run -p gix -- --ignored jj_realistic_needs_to_be_more_clever

This is a workaround for GitoxideLabs#1575.

When run on Windows, the test previously checked if it was running
on CI and skipped its contents. This removes that logic, since the
test no longer runs by default on CI or otherwise.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledged an issue is accepted as shortcoming to be fixed help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants