Skip to content

Commit

Permalink
Simplified path diagnostics test for directories with trailing spaces
Browse files Browse the repository at this point in the history
Committing a fixture directory with a trailing space causes bad bad things to happen on Windows when git tries to restore the fixture file. I've removed the fixture and instead set the test so it generates the fixture when needed.

Work done for #196
  • Loading branch information
atruskie committed Mar 20, 2020
1 parent 894012b commit fbe9a78
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 19 deletions.
31 changes: 28 additions & 3 deletions src/Acoustics.Shared/PathDiagnostics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@ namespace Acoustics.Shared
/// </summary>
public static class PathDiagnostics
{
/// <summary>
/// Checks if a path exists. If it does not it generates a report detailing
/// what aspects of the path do exist and suggests possible alternatives.
/// </summary>
/// <param name="path">The path to check.</param>
/// <param name="report">
/// A report that details errors found in the path if the <paramref name="path"/> does not exist.
/// </param>
/// <param name="root">
/// An optional root to apply to a relative path.
/// Defaults to <see cref="Environment.CurrentDirectory"/> is <paramref name="root"/> is null.
/// </param>
/// <returns>True if the path exists.</returns>
/// <exception cref="ArgumentException">
/// If the supplied <paramref name="root"/> is not itself rooted.
/// </exception>
public static bool PathExistsOrDiff(string path, out PathDiffReport report, string root = null)
{
report = new PathDiffReport();
Expand Down Expand Up @@ -126,7 +142,7 @@ public static bool PathExistsOrDiff(string path, out PathDiffReport report, stri
}

var finalTestPath = builtUpPath;
string lastFragment = string.Empty;
string lastFragment;
if (spaceAtEnd)
{
lastFragment = lastPart.TrimEnd(' ');
Expand Down Expand Up @@ -231,10 +247,19 @@ static bool EndsWithSpaces(string fragment)
}
}

/// <summary>
/// A report on a path.
/// </summary>
public class PathDiffReport
{
public StringBuilder Messages { get; set; } = new StringBuilder(1000);

/// <summary>
/// Gets the messages the comprise the diff report.
/// </summary>
public StringBuilder Messages { get; } = new StringBuilder(1000);

/// <summary>
/// Gets or sets a file system info of the path in question.
/// </summary>
public FileSystemInfo Found { get; set; }
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/Acoustics.Shared/PathUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ private static extern uint GetShortPathName(
string path,
[MarshalAs(UnmanagedType.LPTStr)]
StringBuilder shortPath,
uint shortPathLength
);
uint shortPathLength);
}
}
82 changes: 68 additions & 14 deletions tests/Acoustics.Test/Shared/PathDiagnosticsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ namespace Acoustics.Test.Shared
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using Acoustics.Shared;
using Acoustics.Test.TestHelpers;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Microsoft.Win32.SafeHandles;
using static Acoustics.Test.TestHelpers.PathHelper;

[TestClass]
Expand Down Expand Up @@ -43,7 +45,7 @@ public void ReturnsFalseForNullOrEmpty(string path)

var expected = "Supplied path was null or empty\n";

this.AssertDiff(false, exists, expected, report);
AssertDiff(false, exists, expected, report);
}

[TestMethod]
Expand All @@ -54,7 +56,7 @@ public void ReturnsTrueForPathsThatExist(string path)

var expected = string.Empty;

this.AssertDiff(true, exists, expected, report);
AssertDiff(true, exists, expected, report);
}

[PlatformSpecificTestMethod("Windows")]
Expand All @@ -66,7 +68,7 @@ public void ReturnsTrueForPathsThatExistForwardSlash(string path)

var expected = string.Empty;

this.AssertDiff(true, exists, expected, report);
AssertDiff(true, exists, expected, report);
}

[TestMethod]
Expand All @@ -83,7 +85,7 @@ public void ItCanDetectErrorInFolderName()
{MakeAlternatives(ResolveAssetPath("FSharp"))}
";

this.AssertDiff(false, exists, expected, report);
AssertDiff(false, exists, expected, report);
}

[TestMethod]
Expand All @@ -102,7 +104,7 @@ public void ItCanSuggestFoldersForACompletelyWrongFolder()
{MakeAlternatives(alternatives)}
";

this.AssertDiff(false, exists, expected, report);
AssertDiff(false, exists, expected, report);
}

[TestMethod]
Expand All @@ -121,26 +123,32 @@ public void ItDealsWithErrantSpacesInParentDirectories()
{MakeAlternatives(alternatives)}
";

this.AssertDiff(false, exists, expected, report);
AssertDiff(false, exists, expected, report);
}

[TestMethod]
public void ItDealsWithActualSpacesInParentDirectories()
{
string path = ResolveAssetPath("AFolder", "Example2", "Evil folder3 ", ".gitkeeep");
// arrange
string badDirectory = ResolveAssetPath("AFolder", "Example2", "Evil folder3 ");
MakeIllegalDirectoryWithFileInside(badDirectory, ".gitkeep");

var exists = PathDiagnostics.PathExistsOrDiff(path, out var report);
string badPath = ResolveAssetPath("AFolder", "Example2", "Evil folder3 ", ".gitkeeep");

// act
var exists = PathDiagnostics.PathExistsOrDiff(badPath, out var report);

// assert
var alternatives = ResolveAssetPath("AFolder", "Example2", "Evil folder3 ", ".gitkeep");

var expected = $@"`{path}` does not exist
var expected = $@"`{badPath}` does not exist
Input path differs from real path with character 'e', at column {41 + AssetPathLength}:
{MakeIndicator(41)}
{MakeGoodBad($"AFolder{Slash}Example2{Slash}Evil folder3 {Slash}.gitkee", "ep")}
{MakeAlternatives(alternatives)}
";

this.AssertDiff(false, exists, expected, report);
AssertDiff(false, exists, expected, report);
}

[TestMethod]
Expand Down Expand Up @@ -168,7 +176,7 @@ Input path exists wholly until its end (column {testFragment.Length + AssetPathL
{MakeAlternatives(alternatives)}
";

this.AssertDiff(false, exists, expected, report);
AssertDiff(false, exists, expected, report);
}

[TestMethod]
Expand Down Expand Up @@ -198,7 +206,7 @@ Input path exists wholly until its end (column {testFragment.Length + AssetPathL
{MakeAlternatives(alternatives)}
";

this.AssertDiff(false, exists, expected, report);
AssertDiff(false, exists, expected, report);
}

private static string MakeIndicator(int index, string suffix = "")
Expand All @@ -217,7 +225,53 @@ private static string MakeAlternatives(params string[] alternatives)
return "Here are some alternatives:\n" + alternatives.Select(x => $"\t- {x}").Join("\n");
}

private void AssertDiff(bool expectedExists, bool actualExists, string expected, PathDiagnostics.PathDiffReport report)
/// <summary>
/// Creates a directory with illegal characters in it on Windows.
/// On non-Windows platforms this is equivalent to <see cref="Directory.CreateDirectory"/>.
/// </summary>
/// <param name="path">The directory to create.</param>
/// <param name="child">The name of a file to touch inside the directory.</param>
private static void MakeIllegalDirectoryWithFileInside(string path, string child)
{
if (AppConfigHelper.IsWindows)
{
var directory = @"\\?\" + path;
CreateDirectory(directory, IntPtr.Zero);
SafeFileHandle handle = CreateFileW(
directory + Path.DirectorySeparatorChar + child,
FileAccess.Write,
FileShare.ReadWrite,
IntPtr.Zero,
FileMode.OpenOrCreate,
FileAttributes.Normal,
IntPtr.Zero);
handle.Close();
}
else
{
Directory.CreateDirectory(path);
File.Create(path + Path.DirectorySeparatorChar + child).Dispose();
}
}

[DllImport("kernel32.dll", SetLastError = true)]
private static extern bool CreateDirectory(string lpPathName, IntPtr lpSecurityAttributes);

[DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
private static extern SafeFileHandle CreateFileW(
[MarshalAs(UnmanagedType.LPWStr)] string filename,
[MarshalAs(UnmanagedType.U4)] FileAccess access,
[MarshalAs(UnmanagedType.U4)] FileShare share,
IntPtr securityAttributes,
[MarshalAs(UnmanagedType.U4)] FileMode creationDisposition,
[MarshalAs(UnmanagedType.U4)] FileAttributes flagsAndAttributes,
IntPtr templateFile);

private static void AssertDiff(
bool expectedExists,
bool actualExists,
string expected,
PathDiagnostics.PathDiffReport report)
{
Assert.AreEqual(expectedExists, actualExists);

Expand All @@ -226,4 +280,4 @@ private void AssertDiff(bool expectedExists, bool actualExists, string expected,
Debug.WriteLine(report.Messages.ToString());
}
}
}
}
1 change: 1 addition & 0 deletions tests/Fixtures/AFolder/Example2/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Evil*
Empty file.

0 comments on commit fbe9a78

Please sign in to comment.