improvements #2395
improvements #2395
Conversation
…he read functions.
There was a problem hiding this comment.
Pull request overview
This PR extends status tests and core logic so that unreadable untracked files no longer cause failures during status/rename detection, and adds a fixture helper to execute scripts into writable worktrees.
Changes:
- Generalizes scripted fixture helpers in
tests/toolsto acceptimpl AsRef<Path>and adds a writable, script-executing fixture helper for status tests. - Extends
index_as_worktree_with_renamesto gracefully ignore certain I/O errors (e.g., permission denied) when hashing worktree files, returning a null object ID instead of failing. - Adds a new Unix-only test and fixture exercising unreadable untracked files, plus associated archive ignore configuration.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/tools/src/lib.rs | Broadens scripted fixture helper signatures to take impl AsRef<Path>, enabling use with Path/PathBuf in new tests and fixtures. |
| gix-status/tests/status/mod.rs | Introduces a writable, script-executed fixture helper (fixture_path_rw_slow) for status tests that need to manipulate file permissions. |
| gix-status/tests/status/index_as_worktree_with_renames.rs | Adds a fixture mode enum, wires it into existing tests, and introduces a new Unix-only unreadable_untracked test using a writable executed fixture. |
| gix-status/tests/fixtures/unreadable_untracked.sh | New fixture script that creates a tracked file and an unreadable untracked file to validate that unreadable untracked entries don’t cause failures. |
| gix-status/tests/fixtures/status_many.sh | Modifies the script header, but currently breaks the shebang, which would prevent direct execution in fixture setup. |
| gix-status/tests/fixtures/generated-archives/.gitignore | Ensures archives generated from the new unreadable_untracked.sh fixture are ignored from version control. |
| gix-status/src/index_as_worktree_with_renames/mod.rs | Updates worktree hashing logic to catch specific std::io::ErrorKinds when opening files, log them, and return a null object ID so unreadable untracked files are ignored rather than causing errors. |
Byron
force-pushed
the
improvements
branch
2 times, most recently
from
9bb85e0 to
b53031b
Compare
Instead, just assume it changed.
Leads to failures like this:
thread 'prepare::quoted_command_without_argument_splitting' (19718) panicked at gix-command/tests/command.rs:363:9:
assertion `left == right` failed: looks strange thanks to debug printing, but is the right amount of quotes actually
left: "\"/bin/sh\" \"-c\" \"'ls' \\\"$@\\\"\" \"--\" \"--foo=a b\""
right: "\"/bin/sh\" \"-c\" \"\\'ls\\' \\\"$@\\\"\" \"--\" \"--foo=a b\""
| r#"\'C:\\Users\\O\'\\\'\'Shaughnessy\\with space.exe\' \"$@\""#, | ||
| r#"'C:\\Users\\O'\\''Shaughnessy\\with space.exe' \"$@\""#, | ||
| "--", | ||
| r"--foo=\'a b\'" | ||
| r"--foo='a b'" |
There was a problem hiding this comment.
I'm glad the Debug trait for OsString no longer prints ' characters as \'--it makes things a lot easier to read! Thanks for updating this to adapt to that!
I wonder if we should condition this on the Rust version for a while so the tests can run and pass on previous Rust versions. I might look into doing that--whether it makes sense might depend, in part, on how much else there is in the test suite that relies on very recent Rust.
(It looks like the adapted-to change in the standard library was done in rust-lang/rust#148798 for rust-lang/rust#114583. If I divide this into cases, I can mention that somewhere so it's clear why.)
There was a problem hiding this comment.
Right now the test-suite is susceptible to CI image changes to Rust as well as Git (to a lesser extend), and I think it's fine for now: the failure rate due to that is quite low, with Clippy the number one offender here, but somehow I am not bothered by it.
I am just saying that in case becoming more resilient against it would mean slower CI, rust-toolchain.toml or more complexity for some tests.
Don't fail when untracked files can't be read.
Tasks