From f152628dee6b006c6fc72b15c6ea6bfc3662c63f Mon Sep 17 00:00:00 2001 From: Chris Darroch Date: Mon, 31 Aug 2020 09:24:41 -0700 Subject: [PATCH] FunctionalTests: fix locked pack file fetch test 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 #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 #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. --- .../FetchStepWithoutSharedCacheTests.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Scalar.FunctionalTests/Tests/EnlistmentPerFixture/FetchStepWithoutSharedCacheTests.cs b/Scalar.FunctionalTests/Tests/EnlistmentPerFixture/FetchStepWithoutSharedCacheTests.cs index 6234e68ae7..4a453e3d42 100644 --- a/Scalar.FunctionalTests/Tests/EnlistmentPerFixture/FetchStepWithoutSharedCacheTests.cs +++ b/Scalar.FunctionalTests/Tests/EnlistmentPerFixture/FetchStepWithoutSharedCacheTests.cs @@ -100,8 +100,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(); @@ -118,10 +119,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");