Skip to content

Commit 15cec4e

Browse files
committed
Check that the test affects env::temp_dir() as desired
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.
1 parent 744bb38 commit 15cec4e

File tree

1 file changed

+3
-5
lines changed

1 file changed

+3
-5
lines changed

gix-path/src/env/git/tests.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ fn exe_info_never_from_local_scope() {
384384
let maybe_path = super::exe_info();
385385
assert!(
386386
maybe_path.is_none(),
387-
"Finds no config path if the config would be local"
387+
"Should find no config path if the config would be local"
388388
);
389389
}
390390

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

0 commit comments

Comments
 (0)