Skip to content

Commit

Permalink
Check that the test affects env::temp_dir() as desired
Browse files Browse the repository at this point in the history
This check is probably overzealous in that a path that identifies
the same directory in a different way that is non-equivalent under
path equality would also be acceptable. But a more restrictive
check is simpler, and since we have canonicalized the path and used
it after that for both changing the directory and setting the
environment variables we intend that `env::temp_dir()` will use,
that is unlikely to be a problem.

That it not to say that this cannot break in practice. Rather, it
can break, but if it does, there is a substantial likelihood that
the test is not ensuring the behavior it wishes to check. So to
preserve it as a regression test, failures of this new assertion
should be examined.

This commit also removes some old cruft (commented out test code I
had used while investigating a test bug) and rewords some custom
assertion messages so it is clearer what the expectation is.
  • Loading branch information
EliahKagan committed Aug 28, 2024
1 parent 744bb38 commit 15cec4e
Showing 1 changed file with 3 additions and 5 deletions.
8 changes: 3 additions & 5 deletions gix-path/src/env/git/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ fn exe_info_never_from_local_scope() {
let maybe_path = super::exe_info();
assert!(
maybe_path.is_none(),
"Finds no config path if the config would be local"
"Should find no config path if the config would be local"
);
}

Expand All @@ -403,13 +403,11 @@ fn exe_info_never_from_local_scope_even_if_temp_is_here() {
.set("TMPDIR", repo_str) // Mainly for Unix.
.set("TMP", repo_str) // Mainly for Windows.
.set("TEMP", repo_str); // Mainly for Windows, too.
//assert_eq!(std::env::temp_dir(), repo);
//assert_eq!(std::env::temp_dir(), Path::new("/a/b/c"), "Bogus assertion to debug test");
//std::thread::sleep(std::time::Duration::from_secs(3600));
assert_eq!(std::env::temp_dir(), repo, "It is likely that setting up the test failed");
let maybe_path = super::exe_info();
assert!(
maybe_path.is_none(),
"Finds no config path if the config would be local even in a `/tmp`-like dir"
"Should find no config path if the config would be local even in a `/tmp`-like dir"
);
}

Expand Down

0 comments on commit 15cec4e

Please sign in to comment.