jj icon indicating copy to clipboard operation
jj copied to clipboard

`test_global_opts::test_version` test sometimes fails if run on a merge commit

Open ilyagr opened this issue 1 year ago • 1 comments

Note/update: The test used to be called test_no_subcommand, but is called test_version after #2923. The rest of this description uses the old name in places and might be incrementally updated.


This problem happens non-deterministically for me.

The test (which we should probably rename) checks that jj version outputs a reasonable-looking string.

In my case, running the test on a merge commit sometimes results in an error like this (I changed the assertion to be a little more informative):

thread 'test_no_subcommand' panicked at cli/tests/test_global_opts.rs:70:5:
jj 0.13.0-5c8fb0ab9fbc943f8a8b98028a14638256b71d90432dfcf9fb3eb72a3e01cb599b550bc249d1b216
, sanitized to jj ?.??.?-????????????????????????????????????????????????????????????????????????????????
, does not match expected pattern
stack backtrace:
   0: rust_begin_unwind
             at /rustc/5d3d3479d774754856db2db3e439dfb88ef3c52f/library/std/src/panicking.rs:647:5
   1: core::panicking::panic_fmt
             at /rustc/5d3d3479d774754856db2db3e439dfb88ef3c52f/library/core/src/panicking.rs:72:14
   2: test_global_opts::test_no_subcommand
             at ./tests/test_global_opts.rs:70:5
   3: test_global_opts::test_no_subcommand::{{closure}}
             at ./tests/test_global_opts.rs:46:24
   4: core::ops::function::FnOnce::call_once
             at /rustc/5d3d3479d774754856db2db3e439dfb88ef3c52f/library/core/src/ops/function.rs:250:5
   5: core::ops::function::FnOnce::call_once
             at /rustc/5d3d3479d774754856db2db3e439dfb88ef3c52f/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

The long hexadecimal string here is actually the concatenation of the commit ids of the parents.

The problem is likely in build.rs somewhere. My guess is:

  • There seems to be some non-determinism in which way the build script determines commit ids (via jj or via git). I am not using Nix, so that code path should not be relevant.
  • In one of these, the script concatenates parents' commit ids. I'm not sure whether it would be better to pick one or to concatenate them with a separator (and change the test accordingly).

ilyagr avatar Feb 03 '24 05:02 ilyagr

i wonder if jj --version is occasionally returning an error: https://github.com/martinvonz/jj/blob/7c87fe243c3b14f0321c2bd101af3879770cb90f/cli/build.rs#L60-L71 it would be nice to determine whether to run jj or to run git deterministically based on the presence of .jj or .git; then we can panic on error rather than silently ignoring it. alternatively we could parse the output for "no jj repo in ."

jyn514 avatar Feb 03 '24 06:02 jyn514