Skip to content

Commit

Permalink
Change temp folder name used in installer tests (#45523)
Browse files Browse the repository at this point in the history
Tests in the AppHost.Bundle.Tests assembly seem to randomly fail due to a race condition
with the file system. They try to create separate '0','1','2'... subdirectories to isolate
the published files for each test, but I think what's happening is that files may be
marked for deletion, but then not deleted until a later write. For instance, files in
'2' may be marked for deletion and some may fail a File.Exists check, which leads to
'2' being recreated, at which point deletion may occur, which will cause the current test
to fail due to a concurrent write operation.

This change tries to avoid locking & contention by randomly generating folder names and
using a (hopefully atomically created) lock file to indicate ownership of a particular name.

Fixes #43316
  • Loading branch information
agocke committed Dec 8, 2020
1 parent 69e114c commit cbc10fa
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@

namespace AppHost.Bundle.Tests
{
public class NetCoreApp3CompatModeTests : BundleTestBase, IClassFixture<NetCoreApp3CompatModeTests.SharedTestState>
public class NetCoreApp3CompatModeTests : BundleTestBase, IClassFixture<SingleFileSharedState>
{
private SharedTestState sharedTestState;
private SingleFileSharedState sharedTestState;

public NetCoreApp3CompatModeTests(SharedTestState fixture)
public NetCoreApp3CompatModeTests(SingleFileSharedState fixture)
{
sharedTestState = fixture;
}

[Fact]
public void Bundle_Is_Extracted()
{
var fixture = sharedTestState.SingleFileTestFixture.Copy();
var fixture = sharedTestState.TestFixture.Copy();
UseSingleFileSelfContainedHost(fixture);
Bundler bundler = BundleHelper.BundleApp(fixture, out string singleFile, BundleOptions.BundleAllContent);
var extractionBaseDir = BundleHelper.GetExtractionRootDir(fixture);
Expand All @@ -52,20 +52,5 @@ public void Bundle_Is_Extracted()
.ToArray();
extractionDir.Should().HaveFiles(publishedFiles);
}

public class SharedTestState : SharedTestStateBase, IDisposable
{
public TestProjectFixture SingleFileTestFixture { get; set; }

public SharedTestState()
{
SingleFileTestFixture = PreparePublishedSelfContainedTestProject("SingleFileApiTests");
}

public void Dispose()
{
SingleFileTestFixture.Dispose();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@

namespace AppHost.Bundle.Tests
{
public class SingleFileApiTests : BundleTestBase, IClassFixture<SingleFileApiTests.SharedTestState>
public class SingleFileApiTests : BundleTestBase, IClassFixture<SingleFileSharedState>
{
private SharedTestState sharedTestState;
private SingleFileSharedState sharedTestState;

public SingleFileApiTests(SharedTestState fixture)
public SingleFileApiTests(SingleFileSharedState fixture)
{
sharedTestState = fixture;
}
Expand Down Expand Up @@ -120,23 +120,5 @@ public void AppContext_Native_Search_Dirs_Contains_Bundle_And_Extraction_Dirs()
.And.HaveStdOutContaining(extractionDir)
.And.HaveStdOutContaining(bundleDir);
}

public class SharedTestState : SharedTestStateBase, IDisposable
{
public TestProjectFixture TestFixture { get; set; }

public SharedTestState()
{
// We include mockcoreclr in our project to test native binaries extraction.
string mockCoreClrPath = Path.Combine(RepoDirectories.Artifacts, "corehost_test",
RuntimeInformationExtensions.GetSharedLibraryFileNameForCurrentPlatform("mockcoreclr"));
TestFixture = PreparePublishedSelfContainedTestProject("SingleFileApiTests", $"/p:AddFile={mockCoreClrPath}");
}

public void Dispose()
{
TestFixture.Dispose();
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.IO;
using Microsoft.DotNet.CoreSetup.Test;
using Xunit;
using static AppHost.Bundle.Tests.BundleTestBase;

namespace AppHost.Bundle.Tests
{
public class SingleFileSharedState : SharedTestStateBase, IDisposable
{
public TestProjectFixture TestFixture { get; set; }

public SingleFileSharedState()
{
// We include mockcoreclr in our project to test native binaries extraction.
string mockCoreClrPath = Path.Combine(RepoDirectories.Artifacts, "corehost_test",
RuntimeInformationExtensions.GetSharedLibraryFileNameForCurrentPlatform("mockcoreclr"));
TestFixture = PreparePublishedSelfContainedTestProject("SingleFileApiTests", $"/p:AddFile={mockCoreClrPath}");
}

public void Dispose()
{
TestFixture.Dispose();
}
}
}
52 changes: 37 additions & 15 deletions src/installer/tests/TestUtils/TestArtifact.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable enable

using Microsoft.DotNet.CoreSetup.Test.HostActivation;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Threading;

namespace Microsoft.DotNet.CoreSetup.Test
{
Expand All @@ -22,7 +24,7 @@ public class TestArtifact : IDisposable
{
return _repoDirectoriesProvider.Value.GetTestContextVariable(TestArtifactDirectoryEnvironmentVariable)
?? Path.Combine(AppContext.BaseDirectory, TestArtifactDirectoryEnvironmentVariable);
});
}, isThreadSafe: true);

public static bool PreserveTestRuns() => _preserveTestRuns.Value;
public static string TestArtifactsPath => _testArtifactsPath.Value;
Expand All @@ -32,7 +34,7 @@ public class TestArtifact : IDisposable

private readonly List<TestArtifact> _copies = new List<TestArtifact>();

public TestArtifact(string location, string name = null)
public TestArtifact(string location, string? name = null)
{
Location = location;
Name = name ?? Path.GetFileName(Location);
Expand All @@ -57,7 +59,18 @@ public virtual void Dispose()
{
if (!PreserveTestRuns() && Directory.Exists(Location))
{
Directory.Delete(Location, true);
try
{
Directory.Delete(Location, true);

// Delete lock file last
Debug.Assert(!Directory.Exists(Location));
var lockPath = Directory.GetParent(Location) + ".lock";
File.Delete(lockPath);
} catch (Exception e)
{
Console.WriteLine("delete failed" + e);
}
}

foreach (TestArtifact copy in _copies)
Expand All @@ -68,21 +81,30 @@ public virtual void Dispose()
_copies.Clear();
}

private static readonly object _pathCountLock = new object();
protected static string GetNewTestArtifactPath(string artifactName)
{
int projectCount = 0;
string projectCountDir() => Path.Combine(TestArtifactsPath, projectCount.ToString(), artifactName);

for (; Directory.Exists(projectCountDir()); projectCount++);

lock (_pathCountLock)
Exception? lastException = null;
for (int i = 0; i < 10; i++)
{
string projectDirectory;
for (; Directory.Exists(projectDirectory = projectCountDir()); projectCount++);
FileUtils.EnsureDirectoryExists(projectDirectory);
return projectDirectory;
var parentPath = Path.Combine(TestArtifactsPath, Path.GetRandomFileName());
// Create a lock file next to the target folder
var lockPath = parentPath + ".lock";
var artifactPath = Path.Combine(parentPath, artifactName);
try
{
File.Open(lockPath, FileMode.CreateNew, FileAccess.Write).Dispose();
}
catch (Exception e)
{
// Lock file cannot be created, potential collision
lastException = e;
continue;
}
Directory.CreateDirectory(artifactPath);
return artifactPath;
}
Debug.Assert(lastException != null);
throw lastException;
}

protected static void CopyRecursive(string sourceDirectory, string destinationDirectory, bool overwrite = false)
Expand Down

0 comments on commit cbc10fa

Please sign in to comment.