Skip to content

Commit

Permalink
FunctionalTests: fix locked pack file fetch test
Browse files Browse the repository at this point in the history
The FetchStepFailsWhenItCannotRemoveABadPrefetchPack() functional
test fails at present, as it looks for specific output from the
FetchStep maintenance command in the form of an "Unable to delete"
message.  This string is no longer written to standard output by
the command, however, due to the changes in PR microsoft#295 which introduced
a standard progress meter.  Instead, that string is only written
using this.Context.Tracer.RelatedWarning() in the PerformMaintenance()
method of Scalar.Common.Maintenance.FetchStep, and thus appears only
in the maintenance log files.

The breakage of the test was not noticed because it belongs to the
ExtraCoverage category, and therefore has not been running as part
of the CI suite on Windows because the RunFunctionalTests.bat
script does not supply the --full-suite option.

To resolve the test failure we simply check for the continued
presence of the locked bad pack file after the first fetch attempt
instead of parsing for a specific text string.

The test is also Windows-specific because it depends on an open
file handle with a FileShare.None attribute blocking attempts by
other processes to delete the file, and this only works in C# on
the Windows platform.  On Unix platforms, the C# core libraries
implement file sharing in an advisory manner only; see:

https://github.com/dotnet/runtime/blob/341b8b22280e297daa7d96b4b8a9fa1d40d28aa9/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.Unix.cs#L73-L88

We therefore mark this test as WindowsOnly, since it effectively
does not apply to other platforms.  Note that the previous category
assigned to the test, MacTODO.TestNeedsToLockFile, dates from
VFSForGit and has been migrated to just this one test as a
result of changes in PR microsoft#170.  However, the meaning of that
category varies, as the one other functional test in the category
uses Scalar's FileBasedLock and not C# FileShare settings, so we
clarify the situation by just marking this WindowsOnly.
  • Loading branch information
chrisd8088 committed Sep 3, 2020
1 parent fa76a6d commit 06db5d4
Showing 1 changed file with 5 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,9 @@ public void FetchStepCleansUpBadPrefetchPack()
this.TempPackRoot.ShouldBeADirectory(this.fileSystem).WithNoItems();
}

// WindowsOnly because the test depends on Windows-specific file sharing behavior
[TestCase, Order(4)]
[Category(Categories.MacTODO.TestNeedsToLockFile)]
[Category(Categories.WindowsOnly)]
public void FetchStepFailsWhenItCannotRemoveABadPrefetchPack()
{
this.Enlistment.Unregister();
Expand All @@ -119,10 +120,11 @@ public void FetchStepFailsWhenItCannotRemoveABadPrefetchPack()
// Open a handle to the bad pack that will prevent fetch-commits-and-trees from being able to delete it
using (FileStream stream = new FileStream(badPackPath, FileMode.Open, FileAccess.Read, FileShare.None))
{
string output = this.Enlistment.RunVerb("fetch", failOnError: false);
output.ShouldContain($"Unable to delete {badPackPath}");
this.Enlistment.RunVerb("fetch", failOnError: false);
}

badPackPath.ShouldBeAFile(this.fileSystem).WithContents(badContents);

// After handle is closed fetching commits and trees should succeed
this.Enlistment.RunVerb("fetch");

Expand Down

0 comments on commit 06db5d4

Please sign in to comment.