Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

Commit

Permalink
Always store absolute full path in directory and file path (#9363)
Browse files Browse the repository at this point in the history 
There is no need to store relative path today. But some part of the system does not accept relative path and there is no indication if it is storing full path or not. This is the root cause of https://github.com/dotnet/cli/issues/9319

“someplace” means different full path for Path class on unix and Windows. And the mock file system uses real Path class. Change tests' setup to use essentially “TEMPATH/someplace” instead of  “someplace”
  • Loading branch information
William Li authored Jun 6, 2018
1 parent 2cdef32 commit 2594a6d
Show file tree
Hide file tree
Showing 8 changed files with 104 additions and 58 deletions.
11 changes: 10 additions & 1 deletion src/Microsoft.DotNet.InternalAbstractions/DirectoryPath.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,17 @@ public struct DirectoryPath
{
public string Value { get; }

/// <summary>
/// Create DirectoryPath to repesent a absolute directory path. Note it may not exist.
/// </summary>
/// <param name="value">If the value is not rooted. Path.GetFullPath will be called during the consturctor.</param>
public DirectoryPath(string value)
{
if (!Path.IsPathRooted(value))
{
value = Path.GetFullPath(value);
}

Value = value;
}

Expand Down Expand Up @@ -46,7 +55,7 @@ public override string ToString()

public DirectoryPath GetParentPath()
{
return new DirectoryPath(Directory.GetParent(Path.GetFullPath(Value)).FullName);
return new DirectoryPath(Path.GetDirectoryName(Value));
}
}
}
9 changes: 9 additions & 0 deletions src/Microsoft.DotNet.InternalAbstractions/FilePath.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,17 @@ public struct FilePath
{
public string Value { get; }

/// <summary>
/// Create FilePath to repesent a absolute file path. Note it may not exist.
/// </summary>
/// <param name="value">If the value is not rooted. Path.GetFullPath will be called during the consturctor.</param>
public FilePath(string value)
{
if (!Path.IsPathRooted(value))
{
value = Path.GetFullPath(value);
}

Value = value;
}

Expand Down
11 changes: 11 additions & 0 deletions test/Microsoft.DotNet.Tools.Tests.Utilities/Mock/FileSystemMockBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ public FileSystemMockBuilder AddFiles(string basePath, params string[] files)
return this;
}

/// <summary>
/// Just a "home" means different path on Windows and Unix.
/// Create a platform dependent Temporary directory path and use it to avoid further mis interpretation in
/// later tests. Like "c:/home vs /home". Instead always use Path.Combine(TempraryDirectory, "home")
/// </summary>
internal FileSystemMockBuilder UseCurrentSystemTemporaryDirectory()
{
TemporaryFolder = Path.GetTempPath();
return this;
}

internal IFileSystem Build()
{
return new FileSystemMock(_files, TemporaryFolder);
Expand Down
24 changes: 13 additions & 11 deletions test/dotnet.Tests/BuildServerTests/BuildServerProviderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,37 +81,39 @@ public void GivenNoEnvironmentVariableItUsesTheDefaultPidDirectory()
[Fact]
public void GivenEnvironmentVariableItUsesItForThePidDirectory()
{
const string PidDirectory = "path/to/some/directory";

IFileSystem fileSystem = new FileSystemMockBuilder().UseCurrentSystemTemporaryDirectory().Build();
var pidDirectory = Path.Combine(fileSystem.Directory.CreateTemporaryDirectory().DirectoryPath, "path/to/some/directory");
var provider = new BuildServerProvider(
new FileSystemMockBuilder().Build(),
CreateEnvironmentProviderMock(PidDirectory).Object);
fileSystem,
CreateEnvironmentProviderMock(pidDirectory).Object);

provider
.GetPidFileDirectory()
.Value
.Should()
.Be(PidDirectory);
.Be(pidDirectory);
}

[Fact]
public void GivenARazorPidFileItReturnsARazorBuildServer()
{
const int ProcessId = 1234;
const string ServerPath = "/path/to/rzc.dll";
const string PipeName = "some-pipe-name";

string pidDirectory = Path.GetFullPath("var/pids/build");
string pidFilePath = Path.Combine(pidDirectory, $"{RazorPidFile.FilePrefix}{ProcessId}");

var fileSystemMock = new FileSystemMockBuilder()
.AddFile(
var fileSystemMockBuilder = new FileSystemMockBuilder();
fileSystemMockBuilder.UseCurrentSystemTemporaryDirectory();
var serverPath = Path.Combine(fileSystemMockBuilder.TemporaryFolder, "path/to/rzc.dll");

IFileSystem fileSystemMock = fileSystemMockBuilder.AddFile(
pidFilePath,
$"{ProcessId}{Environment.NewLine}{RazorPidFile.RazorServerType}{Environment.NewLine}{ServerPath}{Environment.NewLine}{PipeName}")
$"{ProcessId}{Environment.NewLine}{RazorPidFile.RazorServerType}{Environment.NewLine}{serverPath}{Environment.NewLine}{PipeName}")
.AddFile(
Path.Combine(pidDirectory, $"{RazorPidFile.FilePrefix}not-a-pid-file"),
"not-a-pid-file")
.Build();
.Build();

var provider = new BuildServerProvider(
fileSystemMock,
Expand All @@ -127,7 +129,7 @@ public void GivenARazorPidFileItReturnsARazorBuildServer()
razorServer.PidFile.Should().NotBeNull();
razorServer.PidFile.Path.Value.Should().Be(pidFilePath);
razorServer.PidFile.ProcessId.Should().Be(ProcessId);
razorServer.PidFile.ServerPath.Value.Should().Be(ServerPath);
razorServer.PidFile.ServerPath.Value.Should().Be(serverPath);
razorServer.PidFile.PipeName.Should().Be(PipeName);
}

Expand Down
16 changes: 10 additions & 6 deletions test/dotnet.Tests/BuildServerTests/RazorServerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public class RazorServerTests
public void GivenAFailedShutdownCommandItThrows()
{
const int ProcessId = 1234;
const string ServerPath = "path/to/rzc.dll";
const string PipeName = "some-pipe-name";
const string ErrorMessage = "error!";

Expand All @@ -33,17 +32,20 @@ public void GivenAFailedShutdownCommandItThrows()

var fileSystemMock = new FileSystemMockBuilder()
.AddFile(pidFilePath, "")
.UseCurrentSystemTemporaryDirectory()
.Build();

fileSystemMock.File.Exists(pidFilePath).Should().BeTrue();

var serverPath = Path.Combine(fileSystemMock.Directory.CreateTemporaryDirectory().DirectoryPath, "path/to/rzc.dll");

var server = new RazorServer(
pidFile: new RazorPidFile(
path: new FilePath(pidFilePath),
processId: ProcessId,
serverPath: new FilePath(ServerPath),
serverPath: new FilePath(serverPath),
pipeName: PipeName),
commandFactory: CreateCommandFactoryMock(ServerPath, PipeName, exitCode: 1, stdErr: ErrorMessage).Object,
commandFactory: CreateCommandFactoryMock(serverPath, PipeName, exitCode: 1, stdErr: ErrorMessage).Object,
fileSystem: fileSystemMock);

Action a = () => server.Shutdown();
Expand All @@ -60,25 +62,27 @@ public void GivenAFailedShutdownCommandItThrows()
public void GivenASuccessfulShutdownItDoesNotThrow()
{
const int ProcessId = 1234;
const string ServerPath = "path/to/rzc.dll";
const string PipeName = "some-pipe-name";

string pidDirectory = Path.GetFullPath("var/pids/build");
string pidFilePath = Path.Combine(pidDirectory, $"{RazorPidFile.FilePrefix}{ProcessId}");

var fileSystemMock = new FileSystemMockBuilder()
.AddFile(pidFilePath, "")
.UseCurrentSystemTemporaryDirectory()
.Build();

fileSystemMock.File.Exists(pidFilePath).Should().BeTrue();

var serverPath = Path.Combine(fileSystemMock.Directory.CreateTemporaryDirectory().DirectoryPath, "path/to/rzc.dll");

var server = new RazorServer(
pidFile: new RazorPidFile(
path: new FilePath(pidFilePath),
processId: ProcessId,
serverPath: new FilePath(ServerPath),
serverPath: new FilePath(serverPath),
pipeName: PipeName),
commandFactory: CreateCommandFactoryMock(ServerPath, PipeName).Object,
commandFactory: CreateCommandFactoryMock(serverPath, PipeName).Object,
fileSystem: fileSystemMock);

server.Shutdown();
Expand Down
48 changes: 26 additions & 22 deletions test/dotnet.Tests/CommandTests/ToolInstallCommandTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,24 +35,28 @@ public class ToolInstallCommandTests
private readonly AppliedOption _appliedCommand;
private readonly ParseResult _parseResult;
private readonly BufferedReporter _reporter;
private const string PathToPlaceShim = "pathToPlace";
private const string PathToPlacePackages = PathToPlaceShim + "pkg";
private readonly string _temporaryDirectory;
private readonly string _pathToPlaceShim;
private readonly string _pathToPlacePackages;
private const string PackageId = "global.tool.console.demo";
private const string PackageVersion = "1.0.4";

public ToolInstallCommandTests()
{
_reporter = new BufferedReporter();
_fileSystem = new FileSystemMockBuilder().Build();
_toolPackageStore = new ToolPackageStoreMock(new DirectoryPath(PathToPlacePackages), _fileSystem);
_fileSystem = new FileSystemMockBuilder().UseCurrentSystemTemporaryDirectory().Build();
_temporaryDirectory = _fileSystem.Directory.CreateTemporaryDirectory().DirectoryPath;
_pathToPlaceShim = Path.Combine(_temporaryDirectory, "pathToPlace");
_pathToPlacePackages = _pathToPlaceShim + "Packages";
_toolPackageStore = new ToolPackageStoreMock(new DirectoryPath(_pathToPlacePackages), _fileSystem);
_createShellShimRepository =
(nonGlobalLocation) => new ShellShimRepository(
new DirectoryPath(PathToPlaceShim),
new DirectoryPath(_pathToPlaceShim),
fileSystem: _fileSystem,
appHostShellShimMaker: new AppHostShellShimMakerMock(_fileSystem),
filePermissionSetter: new NoOpFilePermissionSetter());
_environmentPathInstructionMock =
new EnvironmentPathInstructionMock(_reporter, PathToPlaceShim);
new EnvironmentPathInstructionMock(_reporter, _pathToPlaceShim);
_createToolPackageStoreAndInstaller = (_) => (_toolPackageStore, CreateToolPackageInstaller());

ParseResult result = Parser.Instance.Parse($"dotnet tool install -g {PackageId}");
Expand Down Expand Up @@ -197,7 +201,7 @@ public void GivenFailedPackageInstallWhenRunWithPackageIdItShouldFail()
Environment.NewLine +
string.Format(LocalizableStrings.ToolInstallationFailedWithRestoreGuidance, PackageId));

_fileSystem.Directory.Exists(Path.Combine(PathToPlacePackages, PackageId)).Should().BeFalse();
_fileSystem.Directory.Exists(Path.Combine(_pathToPlacePackages, PackageId)).Should().BeFalse();
}

[Fact]
Expand All @@ -220,7 +224,7 @@ public void GivenCreateShimItShouldHaveNoBrokenFolderOnDisk()
CommonLocalizableStrings.ShellShimConflict,
ProjectRestorerMock.FakeCommandName));

_fileSystem.Directory.Exists(Path.Combine(PathToPlacePackages, PackageId)).Should().BeFalse();
_fileSystem.Directory.Exists(Path.Combine(_pathToPlacePackages, PackageId)).Should().BeFalse();
}

[Fact]
Expand Down Expand Up @@ -257,7 +261,7 @@ public void WhenRunWithPackageIdItShouldShowSuccessMessage()
_parseResult,
_createToolPackageStoreAndInstaller,
_createShellShimRepository,
new EnvironmentPathInstructionMock(_reporter, PathToPlaceShim, true),
new EnvironmentPathInstructionMock(_reporter, _pathToPlaceShim, true),
_reporter);

installCommand.Execute().Should().Be(0);
Expand All @@ -284,7 +288,7 @@ public void WhenRunWithInvalidVersionItShouldThrow()
result,
_createToolPackageStoreAndInstaller,
_createShellShimRepository,
new EnvironmentPathInstructionMock(_reporter, PathToPlaceShim, true),
new EnvironmentPathInstructionMock(_reporter, _pathToPlaceShim, true),
_reporter);

Action action = () => installCommand.Execute();
Expand All @@ -307,7 +311,7 @@ public void WhenRunWithExactVersionItShouldSucceed()
result,
_createToolPackageStoreAndInstaller,
_createShellShimRepository,
new EnvironmentPathInstructionMock(_reporter, PathToPlaceShim, true),
new EnvironmentPathInstructionMock(_reporter, _pathToPlaceShim, true),
_reporter);

installCommand.Execute().Should().Be(0);
Expand All @@ -333,7 +337,7 @@ public void WhenRunWithValidVersionRangeItShouldSucceed()
result,
_createToolPackageStoreAndInstaller,
_createShellShimRepository,
new EnvironmentPathInstructionMock(_reporter, PathToPlaceShim, true),
new EnvironmentPathInstructionMock(_reporter, _pathToPlaceShim, true),
_reporter);

installCommand.Execute().Should().Be(0);
Expand All @@ -359,7 +363,7 @@ public void WhenRunWithoutAMatchingRangeItShouldFail()
result,
_createToolPackageStoreAndInstaller,
_createShellShimRepository,
new EnvironmentPathInstructionMock(_reporter, PathToPlaceShim, true),
new EnvironmentPathInstructionMock(_reporter, _pathToPlaceShim, true),
_reporter);

Action a = () => installCommand.Execute();
Expand All @@ -369,7 +373,7 @@ public void WhenRunWithoutAMatchingRangeItShouldFail()
LocalizableStrings.ToolInstallationRestoreFailed +
Environment.NewLine + string.Format(LocalizableStrings.ToolInstallationFailedWithRestoreGuidance, PackageId));

_fileSystem.Directory.Exists(Path.Combine(PathToPlacePackages, PackageId)).Should().BeFalse();
_fileSystem.Directory.Exists(Path.Combine(_pathToPlacePackages, PackageId)).Should().BeFalse();
}

[Fact]
Expand All @@ -383,7 +387,7 @@ public void WhenRunWithValidVersionWildcardItShouldSucceed()
result,
_createToolPackageStoreAndInstaller,
_createShellShimRepository,
new EnvironmentPathInstructionMock(_reporter, PathToPlaceShim, true),
new EnvironmentPathInstructionMock(_reporter, _pathToPlaceShim, true),
_reporter);

installCommand.Execute().Should().Be(0);
Expand Down Expand Up @@ -411,7 +415,7 @@ public void WhenRunWithBothGlobalAndToolPathShowErrorMessage()
parseResult,
_createToolPackageStoreAndInstaller,
_createShellShimRepository,
new EnvironmentPathInstructionMock(_reporter, PathToPlaceShim, true),
new EnvironmentPathInstructionMock(_reporter, _pathToPlaceShim, true),
_reporter);

Action a = () => installCommand.Execute();
Expand All @@ -433,7 +437,7 @@ public void WhenRunWithNeitherOfGlobalNorToolPathShowErrorMessage()
parseResult,
_createToolPackageStoreAndInstaller,
_createShellShimRepository,
new EnvironmentPathInstructionMock(_reporter, PathToPlaceShim, true),
new EnvironmentPathInstructionMock(_reporter, _pathToPlaceShim, true),
_reporter);

Action a = () => installCommand.Execute();
Expand All @@ -454,7 +458,7 @@ public void WhenRunWithPackageIdAndBinPathItShouldNoteHaveEnvironmentPathInstruc
parseResult,
_createToolPackageStoreAndInstaller,
_createShellShimRepository,
new EnvironmentPathInstructionMock(_reporter, PathToPlaceShim),
new EnvironmentPathInstructionMock(_reporter, _pathToPlaceShim),
_reporter);

installCommand.Execute().Should().Be(0);
Expand All @@ -466,7 +470,7 @@ public void WhenRunWithPackageIdAndBinPathItShouldNoteHaveEnvironmentPathInstruc
public void AndPackagedShimIsProvidedWhenRunWithPackageIdItCreateShimUsingPackagedShim()
{
var extension = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? ".exe" : string.Empty;
var prepackagedShimPath = "packagedShimDirectory/" + ProjectRestorerMock.FakeCommandName + extension;
var prepackagedShimPath = Path.Combine (_temporaryDirectory, ProjectRestorerMock.FakeCommandName + extension);
var tokenToIdentifyPackagedShim = "packagedShim";
_fileSystem.File.WriteAllText(prepackagedShimPath, tokenToIdentifyPackagedShim);

Expand All @@ -490,7 +494,7 @@ [new PackageId(PackageId)] = new[] {new FilePath(prepackagedShimPath)}
fileSystem: _fileSystem,
reporter: _reporter))),
_createShellShimRepository,
new EnvironmentPathInstructionMock(_reporter, PathToPlaceShim),
new EnvironmentPathInstructionMock(_reporter, _pathToPlaceShim),
_reporter);

installCommand.Execute().Should().Be(0);
Expand All @@ -512,11 +516,11 @@ private IToolPackageInstaller CreateToolPackageInstaller(
installCallback: installCallback);
}

private static string ExpectedCommandPath()
private string ExpectedCommandPath()
{
var extension = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? ".exe" : string.Empty;
return Path.Combine(
"pathToPlace",
_pathToPlaceShim,
ProjectRestorerMock.FakeCommandName + extension);
}

Expand Down
Loading

0 comments on commit 2594a6d

Please sign in to comment.