search

assert-rs / snapbox

Snapshot testing for a herd of CLI tests
docs.rs/trycmd
Apache License 2.0
138 stars 18 forks source link

Going from 0.15.2 to 0.15.4 broke our tests #354

Open cafkafk opened 4 months ago

cafkafk commented 4 months ago

See https://github.com/eza-community/eza/pull/1005

For testing, normally, we run tests with just itest inside of our repo, which requires nix.

Example of breakage:

eza> Testing tests/ptests/ptest_a689ab7558716dda.toml ... failed
eza> Exit: success
eza> ---- expected: stdout
eza> ++++ actual:   stdout
eza>    1      - 8;;file:///build/source/tests/test_dir/git/git8;;/
eza>    2      - 8;;file:///build/source/tests/test_dir/grid/grid8;;/
eza>    3      - 8;;file:///build/source/tests/test_dir/group/group8;;/
eza>    4      - 8;;file:///build/source/tests/test_dir/icons/icons8;;/
eza>    5      - 8;;file:///build/source/tests/test_dir/perms/perms8;;/
eza>    6      - 8;;file:///build/source/tests/test_dir/size/size8;;/
eza>    7      - 8;;file:///build/source/tests/test_dir/specials/specials8;;/
eza>    8      - 8;;file:///build/source/tests/test_dir/symlinks/symlinks8;;/
eza>    9      - 8;;file:///build/source/tests/test_dir/time/time8;;/
eza>         1 + 8;;file://[CWD]/tests/test_dir/git/git8;;/
eza>         2 + 8;;file://[CWD]/tests/test_dir/grid/grid8;;/
eza>         3 + 8;;file://[CWD]/tests/test_dir/group/group8;;/
eza>         4 + 8;;file://[CWD]/tests/test_dir/icons/icons8;;/
eza>         5 + 8;;file://[CWD]/tests/test_dir/perms/perms8;;/
eza>         6 + 8;;file://[CWD]/tests/test_dir/size/size8;;/
eza>         7 + 8;;file://[CWD]/tests/test_dir/specials/specials8;;/
eza>         8 + 8;;file://[CWD]/tests/test_dir/symlinks/symlinks8;;/
eza>         9 + 8;;file://[CWD]/tests/test_dir/time/time8;;/
eza> stderr:
eza> 
eza> Testing tests/ptests/ptest_4974d70325cb7550.toml ... ok
eza> Testing tests/ptests/ptest_91d7b6efe549ede0.toml ... ok
eza> Testing tests/ptests/ptest_a6bbf53a066c588e.toml ... ok
eza> Testing tests/ptests/ptest_98e04e3185e9174c.toml ... ok
eza> Testing tests/ptests/ptest_4e899e9b065acc8f.toml ... ok
eza> Testing tests/ptests/ptest_5a8530dc7b091286.toml ... ok
eza> Testing tests/ptests/ptest_89146337fb6b0967.toml ... ok
eza> Testing tests/ptests/ptest_4805a91da5df26.toml ... ok
eza> Testing tests/ptests/ptest_4b30f7de50929327.toml ... failed
eza> Exit: success
eza> ---- expected: stdout
eza> ++++ actual:   stdout
eza>    1      - /build/source/tests/test_dir/git
eza>    2      - /build/source/tests/test_dir/grid
eza>    3      - /build/source/tests/test_dir/group
eza>    4      - /build/source/tests/test_dir/icons
eza>    5      - /build/source/tests/test_dir/perms
eza>    6      - /build/source/tests/test_dir/size
eza>    7      - /build/source/tests/test_dir/specials
eza>    8      - /build/source/tests/test_dir/symlinks
eza>    9      - /build/source/tests/test_dir/time
eza>         1 + [CWD]/tests/test_dir/git
eza>         2 + [CWD]/tests/test_dir/grid
eza>         3 + [CWD]/tests/test_dir/group
eza>         4 + [CWD]/tests/test_dir/icons
eza>         5 + [CWD]/tests/test_dir/perms
eza>         6 + [CWD]/tests/test_dir/size
eza>         7 + [CWD]/tests/test_dir/specials
eza>         8 + [CWD]/tests/test_dir/symlinks
eza>         9 + [CWD]/tests/test_dir/time
eza> stderr:
eza>
epage commented 4 months ago

I assume you are reporting this against trycmd?

Do you have a minimal reproduction case I can work with for identifying the root cause?

cafkafk commented 4 months ago

I assume you are reporting this against trycmd?

Yup, I think that's reasonable consider the only change was the version bump leading to a breakage, if I need to move this issue lmk, I don't understand the snapbox internals >_>

Do you have a minimal reproduction case I can work with for identifying the root cause?

The problem is our tests rely on some nix sandboxing assumptions, and tests will completely break without having a nix environment. Entering the repository, typing nix develop, and running the just recipe just itest would maybe be the easiest, but again this does require having nix installed.

I know that's pretty convoluted, I'm sorry I don't have a simpler way to do this. It may be possible to do it without nix by manipulating timestamps, but there may be other behavior that also breaks in strange ways.

epage commented 4 months ago

The challenge I'm having is I can't see why this is failing and without any context, the new output seems more correct. If /build/source was the CWD for the tests being run, I believe that substitution should have been happening.

I went back and audited this. All that changed in these versions was internals as we changed the underlying snapbox package. For reference, the command I ran was

$ git diff 72241ddc76e3ffbbb86e84e370492ea3c03d11cd -- .

(see also https://github.com/assert-rs/snapbox/compare/v0.15.2...trycmd-v0.15.4)

cafkafk commented 2 months ago

The actual issue that has occurred seems to be that /build/source isn't replaced with [CWD] when generated in the Nix sandbox, but when someone runs tests without being inside of the sandbox, i.e. normal cargo test in the repo, [CWD] is substituted seemingly correctly.

Perhaps the current working directory of the sandbox was root? Does that not get substituted for [CWD]?

I can't explicitly find something in the version bump that should be the culprit either, although it is odd it started appearing after. Right now my workaround is doing fd -e stdout -e stderr -H -t file tests.

epage commented 2 months ago

Regarding the handling of [CWD], the most relevant changes I see are