The sound distributed version control system

#532 Have `cargo test` compile again

Closed 2 days ago
zj on September 13, 2021

When executing cargo test compilation fails, which is worst than failing tests as now one can’t even validate their changes breaks more tests. As such this discussion aims to collect patches out there to resolve this, and have cargo test compile again.

ammkrn on September 13, 2021

A good intermediate measure (between now and all tests eventually passing) might be to identify a subset of tests that can currently be run by contributors before a push to determine whether or not their change is likely to break something.

zj on September 13, 2021

As far as I’m aware it’s not possible to run any tests as long as it doesn’t compile, correct? Or am I missing a trick?

potocpav added a change on September 13, 2021
Fix test compilation by FHRXP5Jnb2MWLDrPrnLnkN2ryWcGCo6CRr1dXR9FW2YA,
OEKRRU6OMDAHD3UT2L56WTAVYMJUZM7JBTRJBJR6MAV7EXBPTHYQC
main
ammkrn on September 13, 2021

I think you’re right. I guess for tests that are either “aspirational” and don’t yet pass or tests that are complex/would be very time consuming to fix, you they would need to be commented out. The main thing is that I think your concern about contributors not having tests to run before submitting a patch is valid.

Looking at the state of the repo again, I’m not sure how many of the tests are actually broken now (cargo test shows 11 hard errors, but nominally fixing those might reveal more). The last time I checked prior to today it was a much larger number, so I figured fixing all of the tests would take a while.

potocpav on September 13, 2021

This should do it? I fixed everything by local reasoning, since I am unfamiliar with the code-base. Hope I didn’t break anything.

EDIT: the test-suite actually passes, which is super nice.

ammkrn on September 14, 2021

@potocpav

Works for me too, thanks for doing that.

zj on September 15, 2021

For me I ended up with 4 failures:

failures:

---- tests::filesystem::filesystem stdout ----
Error: Changestore error: Is a directory (os error 21)
thread 'tests::filesystem::filesystem' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/test/src/lib.rs:194:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- tests::filesystem::overwrite_dead_symlink stdout ----
Error: Changestore error: Is a directory (os error 21)
thread 'tests::filesystem::overwrite_dead_symlink' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/test/src/lib.rs:194:5

---- tests::filesystem::record_dead_symlink stdout ----
Error: Changestore error: Is a directory (os error 21)
thread 'tests::filesystem::record_dead_symlink' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/test/src/lib.rs:194:5

---- tests::filesystem::symlink stdout ----
Error: Changestore error: Is a directory (os error 21)
thread 'tests::filesystem::symlink' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/test/src/lib.rs:194:5

Apple M1; so Darwin, might be specific to my machine.

Either way, this patch is a great step in the right direction. @pmeunier could you apply to main?

potocpav on September 15, 2021

@zj It’s already applied :-) You can tell by the small tick-mark next to “main” in the change banner

zj 2 days ago

Closing this discussion, thanks everyone!

zj closed this discussion 2 days ago