History

Fix bug in new test where temp dir should be a repo

The problem was that the path used for environment variables that inform `env::temp_dir()` was a relative path. Although `temp_dir()` itself will return an absolute path (resolved from it), the test changes the current directory. This change occurred between when the relative path was obtained by the test code and when it was resolved by the code under test. The path was therefore resolved to a different location (the last few path components were repeated) that never exists. Since it didn't exist, the `git config -l` command failed, and no installation config file path was returned, causing the test to wrongly pass (since `None` is the correct result when only local scope configuration files are available). The fix is to use an absolute path. In addition, this canonicalizes the path rather than merely resolving it, and does so even before changing directory to it, so that if `temp_dir()` "fixes up" the path in ways beyond resolving it, such as resolving symlinks, the test is less likely to wrongly pass. The test now rightly fails, since the hardening it tests for has not yet been implemented in the code under test. Further changes to the test are still warranted, at least to clean it up, and possibly also to make it slightly more robust.

Eliah Kagan committed 2 years ago

Commit 744bb381 parent 287f2671 file · +7 −1Browse files

Unified · Split

gix-path/src/env/git/tests.rs+7−1
@@ -391,7 +391,10 @@ fn exe_info_never_from_local_scope() { · 390 unmodified lines
391#[test]391#[test]
392#[serial]392#[serial]
393fn exe_info_never_from_local_scope_even_if_temp_is_here() {393fn exe_info_never_from_local_scope_even_if_temp_is_here() {
394 let repo = gix_testtools::scripted_fixture_read_only("local_config.sh").expect("script succeeds");394 let repo = gix_testtools::scripted_fixture_read_only("local_config.sh")
395 .expect("script succeeds")
396 .canonicalize()
397 .expect("path is valid and exists");
395 let repo_str = repo.to_str().expect("valid Unicode");398 let repo_str = repo.to_str().expect("valid Unicode");
396 let _cwd = gix_testtools::set_current_dir(&repo).expect("can change to repo dir");399 let _cwd = gix_testtools::set_current_dir(&repo).expect("can change to repo dir");
397 let _env = gix_testtools::Env::new()400 let _env = gix_testtools::Env::new()
@@ -400,6 +403,9 @@ fn exe_info_never_from_local_scope_even_if_temp_is_here() { · 2 unmodified lines
400 .set("TMPDIR", repo_str) // Mainly for Unix.403 .set("TMPDIR", repo_str) // Mainly for Unix.
401 .set("TMP", repo_str) // Mainly for Windows.404 .set("TMP", repo_str) // Mainly for Windows.
402 .set("TEMP", repo_str); // Mainly for Windows, too.405 .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));
403 let maybe_path = super::exe_info();409 let maybe_path = super::exe_info();
404 assert!(410 assert!(
405 maybe_path.is_none(),411 maybe_path.is_none(),