diff --git a/src/DiffEngine.Tests/GuardTests.cs b/src/DiffEngine.Tests/GuardTests.cs new file mode 100644 index 00000000..9b834514 --- /dev/null +++ b/src/DiffEngine.Tests/GuardTests.cs @@ -0,0 +1,27 @@ +public class GuardTests +{ + /// + /// FileExists passed its arguments to AgainstEmpty the wrong way round, so the empty check was + /// run against the literal parameter name - which is never empty. An empty path therefore fell + /// through to "File not found. Path: " with no ParamName on it, naming nothing at all. + /// + [Test] + [Arguments("")] + [Arguments(" ")] + public async Task FileExistsRejectsAnEmptyPathByName(string path) + { + var exception = await Assert.That(() => Guard.FileExists(path, "tempFile")) + .Throws(); + + await Assert.That(exception!.ParamName).IsEqualTo("tempFile"); + } + + [Test] + public async Task FileExistsStillReportsAMissingFile() + { + var missing = Path.Combine(Path.GetTempPath(), $"missing{Guid.NewGuid():N}.txt"); + + await Assert.That(() => Guard.FileExists(missing, "tempFile")) + .Throws(); + } +} diff --git a/src/DiffEngine/Guard.cs b/src/DiffEngine/Guard.cs index b6360b70..615bdf85 100644 --- a/src/DiffEngine/Guard.cs +++ b/src/DiffEngine/Guard.cs @@ -10,7 +10,9 @@ public static void AgainstNegative(int value, string argumentName) public static void FileExists(string path, string argumentName) { - AgainstEmpty(argumentName, path); + // (value, name), not (name, value). Reversed, this validated the literal parameter name - + // never empty - so an empty path fell through to the message below with no ParamName on it + AgainstEmpty(path, argumentName); if (!File.Exists(path)) { throw new ArgumentException($"File not found. Path: {path}"); diff --git a/src/DiffEngine/OsSettingsResolver.cs b/src/DiffEngine/OsSettingsResolver.cs index b1585ed1..8d5fb518 100644 --- a/src/DiffEngine/OsSettingsResolver.cs +++ b/src/DiffEngine/OsSettingsResolver.cs @@ -4,7 +4,10 @@ static class OsSettingsResolver static OsSettingsResolver() { - var pathVariable = Environment.GetEnvironmentVariable("PATH")!; + // An unset PATH is a NullReferenceException in a static constructor, and so permanent + // for the process. `env -i` and some service launchers really do start a process without + // one; nothing on PATH simply means no tool is found that way + var pathVariable = Environment.GetEnvironmentVariable("PATH") ?? ""; if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) {