Skip to content

Commit

Permalink
Test that env for fixture scripts has only command-scope config
Browse files Browse the repository at this point in the history
It doesn't have to have any configuration variables set in the
command scope, though in practice it always should (with versions
of `git` with which `gix-testtools` is compatible) because we are
setting them with `GIT_CONFIG_{COUNT,KEY,VALUE}`, which, like `-c`,
sets configuration variables in the command scope.

But it must not have any configuration variables set in other
scopes. Of course, actual fixture scripts, when run, most often
create Git repositories, in which case there will in practice
almost always be local scope configuration for the script.

The main point here is to make sure we are omitting variables from
the global and system scopes, as we have already been doing (and
testing for), and also omitting them from the other high scopes,
such as the "unknown" scope associated with an Apple Git
installation that is higher than the system scope and unaffected
by `GIT_CONFIG_SYSTEM` but affected by `GIT_CONFIG_NOSYSTEM`.

Like the tests that preceded it, this test creates a new empty
temporary directory to set as the CWD of the test `git` command
configured by `configure_command`. As such, there should be no
local scope configuration, because this directory should be a
subdirectory of a system `/tmp`-like directory.

A `/tmp`-like directory can technically be a Git repository, and
can even contribute configuration if the repository is owned by the
current user or something is keeping `safe.directory` protections
from excluding it.

When the goal is to *get* configuration from scopes higher than the
local scope, it is worth taking steps to prevent this (which for
`gix-path` is achieved by the combination of GitoxideLabs#1523 and GitoxideLabs#1567).

But in the test suite, temporary directories should preferably be
in locations that are only `git` repositories (owned by the curent
user) in the unusual situation that this is desired, and supporting
tooling such as `git` should be at recent enough versions to really
support the usage that the test suite makes of it.

Furthermore, while it is possible to clear the local scope in this
new test -- such as by using, as the command's CWD, a subdirectory
of a directory that is, in the command's environment, prepended to
`GIT_CEILING_DIRECTORIES` -- this would tend to hide problems in
the actual `gix-testtools` code that this is testing.

If any such measure needs to be taken, it would be better done in
code that uses `configure_command` (or in `configure_command`
itself it it is widely needed and generally acceptable), rather
than in tests of `configure_command`.

For these reasons, the test is, at least for now, deliberately
written in such a way that it will fail if the directory we get
from `tempfile::TempDir::new()` is somehow a Git repository.

This commit consolidates the two preceding test cases into this
one. So now there is a single test that `configure-command` both:

- Excludes global and system scope config, as it already did.
- Also excludes other high-scoped config, which it doesn't yet do.

Thus, this new test is expected to fail on most macOS systems
(where `git` is Apple Git and the installation-associated "unknown"
scope configuration file is nonempty), until that is fixed.
  • Loading branch information
EliahKagan committed Sep 1, 2024
1 parent 6f128dd commit d576b32
Showing 1 changed file with 16 additions and 22 deletions.
38 changes: 16 additions & 22 deletions tests/tools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -898,43 +898,37 @@ mod tests {
} else {
&[dir.join(SCOPE_ENV_VALUE), dir.join("-"), dir.join(":")]
};

// Create the files.
for path in paths {
std::fs::write(path, CONFIG_DATA).expect("can write contents");
}

// Verify the files. This is mostly to show we really made a `\\?\...\NUL` on Windows.
for path in paths {
let buf = std::fs::read(path).expect("the file really exists");
assert_eq!(buf, CONFIG_DATA, "File {path:?} should be created");
assert_eq!(buf, CONFIG_DATA, "{path:?} should be a config file");
}
}

fn check_configure_clears_scope(scope_env_key: &str, scope_option: &str) {
#[test]
fn configure_command_clears_external_config() {
let temp = tempfile::TempDir::new().expect("can create temp dir");
let dir = temp.path();
populate_ad_hoc_config_files(dir);
populate_ad_hoc_config_files(temp.path());

let mut cmd = std::process::Command::new("git");
cmd.env(scope_env_key, SCOPE_ENV_VALUE); // configure_command() should override it.
let args = ["config", "-l", "--show-origin", scope_option].map(String::from);
configure_command(&mut cmd, &args, dir);
let args = ["config", "-l", "--show-origin"].map(String::from);
cmd.env("GIT_CONFIG_SYSTEM", SCOPE_ENV_VALUE);
cmd.env("GIT_CONFIG_GLOBAL", SCOPE_ENV_VALUE);
configure_command(&mut cmd, &args, temp.path());

let output = cmd.output().expect("can run git");
let stdout = output.stdout.to_str().expect("valid UTF-8");
let lines: Vec<_> = output.stdout
.to_str()
.expect("valid UTF-8")
.lines()
.filter(|line| !line.starts_with("command line:\t"))
.collect();
let status = output.status.code().expect("terminated normally");
assert_eq!(stdout, "", "should be no config variables to display");
assert_eq!(status, 0, "reading the config should nonetheless succeed");
}

#[test]
fn configure_command_clears_system_scope() {
check_configure_clears_scope("GIT_CONFIG_SYSTEM", "--system");
}

#[test]
fn configure_command_clears_global_scope() {
check_configure_clears_scope("GIT_CONFIG_GLOBAL", "--global");
assert_eq!(lines, Vec::<&str>::new(), "should be no config variables from files");
assert_eq!(status, 0, "reading the config should succeed");
}
}

0 comments on commit d576b32

Please sign in to comment.