Add installer tests project #46927

NikolaMilosavljevic · 2025-02-18T23:16:09Z

This PR introduces test project for installer tests. It does not hook it up to build infra. There will be another PR soon that will introduce a job/step in VMR Validation stage that will run installer tests for both x64 and arm64 installers.

Originally the plan was to run these tests as part of the mainline build job, but we are running that job as cross-build on x64 host, which precludes us from testing arm64 installers.

Some notes about the PR contents:

There is one RPM and one Debian test platform - this can be expanded as needed
Debian installation uses --force-all to work around issues in [VMR] aspnetcore packages produced in VMR have incorrect prerelease versioning source-build#4895 - the fix is in PR and this will be removed in the near future. I opted not to include that fix as a patch, but that is a possibility
Consumers can specify if they want to test just Deb or Rpm or both
DockerHelper.cs was borrowed from dotnet-docker repo: https://github.com/dotnet/dotnet-docker/blob/main/tests/Microsoft.DotNet.Docker.Tests/DockerfileHelper.cs. I have removed many unused methods
Only Linux installer tests are being added today, Windows and MacOS installers will be considered in the future

Copilot

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/Microsoft.DotNet.Installer.Tests.csproj: Language not supported

Comments suppressed due to low confidence (3)

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/DockerHelper.cs:150

The outputHelper parameter is marked as non-nullable but is being passed as null in some cases, which could lead to a NullReferenceException.

Func<string, ITestOutputHelper, (Process Process, string StdOut, string StdErr)> executor)

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs:101

Assert.Fail is not a valid method in xUnit. Use Assert.True(false, "Test failed for {image}: {ex}") instead.

Assert.Fail("Test failed for {image}: {ex}");

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs:229

Assert.Fail is not a valid method in xUnit. Use Assert.True(false, "Build failed: {output}") instead.

Assert.Fail("Build failed: {output}");

Copilot · 2025-02-18T23:17:25Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+        string? scenarioTestsPackage = Directory.GetFiles(nugetPackagesDir, "Microsoft.DotNet.ScenarioTests.SdkTemplateTests*.nupkg", SearchOption.AllDirectories).FirstOrDefault();
+        if (scenarioTestsPackage == null)
+        {
+            Assert.Fail("Scenario tests package not found");


Assert.Fail is not a valid method in xUnit. Use Assert.True(false, "Scenario tests package not found") instead.

Suggested change

Assert.Fail("Scenario tests package not found");

Assert.True(false, "Scenario tests package not found");

Not true - Assert.Fail exists and is new in v2 2.5, see https://xunit.net/docs/comparisons

Copilot · 2025-02-18T23:17:26Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+            }
+            else
+            {
+                Assert.Fail("Test summary not found");


Assert.Fail is not a valid method in xUnit. Use Assert.True(false, "Test summary not found") instead.

Suggested change

Assert.Fail("Test summary not found");

Assert.True(false, "Test summary not found");

Not true - Assert.Fail exists and is new in v2 2.5, see https://xunit.net/docs/comparisons

jkoritzinsky · 2025-02-19T02:04:20Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+
+    private readonly string[] RpmDistroImages =
+    [
+        "mcr.microsoft.com/dotnet-buildtools/prereqs:fedora-40"


We don't ship the Fedora packages any more (and I'm actually in the process of removing the runtime-deps package in another PR). Can we instead use Azure Linux 3.0 or OpenSuSE Leap 15.6 here? Those are both RPM distros where we currently ship the packages.

Also, could we use the shipping "runtime-deps" images as our baseline instead of the buildtools-prereqs images? The shipping images better represent our customer scenarios.

Thanks - will fix.

A deps image would likely be this one mcr.microsoft.com/dotnet/runtime-deps:9.0-azurelinux3.0 as we don't have 10.0 images yet. I don't think we want the OS base image like mcr.microsoft.com/azurelinux/base/core:3.0

I think we should have 10.0 images soon (when P1 comes out), but the 9.0 ones should be good for now.

Fixed with 054366b

jkoritzinsky · 2025-02-19T02:05:32Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+
+    private readonly string[] DebDistroImages =
+    [
+        "mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-24.04"


We don't ship the Ubuntu packages any more (and I'm actually in the process of removing the Ubuntu support from the runtime-deps package in another PR). Can we instead use Debian stable here? Debian is the only disto that we ship our Deb packages for now.

Also, could we use the shipping "runtime-deps" image as our baseline instead of the buildtools-prereqs image? The shipping images better represent our customer scenarios.

Thanks - yeah makes sense, I'll update this.

Deps image would likely be this one: mcr.microsoft.com/dotnet/runtime-deps:9.0-bookworm-slim

Fixed with 054366b

jkoritzinsky · 2025-02-19T02:07:25Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+            string depsId = "fedora";
+            if (baseImage.Contains("fedora"))
+            {
+                depsId = "fedora";
+            }
+            else if (baseImage.Contains("centos"))
+            {
+                depsId = "centos";
+            }
+            else if (baseImage.Contains("rhel"))
+            {
+                depsId = "rhel";
+            }
+            else if (baseImage.Contains("opensuse"))


I'm removing these deps packages, so we don't need these checks here.

Suggested change

string depsId = "fedora";

if (baseImage.Contains("fedora"))

{

depsId = "fedora";

}

else if (baseImage.Contains("centos"))

{

depsId = "centos";

}

else if (baseImage.Contains("rhel"))

{

depsId = "rhel";

}

else if (baseImage.Contains("opensuse"))

string? depsId = null;

if (baseImage.Contains("opensuse"

Thanks, I'll incorporate this in an offline change, along other proposed changes.

Fixed with 054366b

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

jkoritzinsky · 2025-02-19T02:14:50Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+        sb.Append("RUN");
+
+        // TODO: remove --force-all when aspnet package versioning issue have been resolved - https://github.com/dotnet/source-build/issues/4895
+        string packageInstallationCommand = distroType == DistroType.Deb ? "dpkg -i --force-all" : "rpm -i";


Could we construct these tests to point to a fake local package feed and install the packages via apt/tdnf/zypper instead of dpkg/rpm directly?

Doing so would let us better test actual end user scenarios

That's something we could consider in the future. It's not as trivial to now add that to current tests, at least not as easy as adding a local NuGet store. It would be easy to reuse most of the code for those tests.

For now we're automating current manual testing of RPMs an DEBs, in the same way they are executed.

Utilizing correct installation order also verifies the expected package dependencies.

MichaelSimons · 2025-02-19T19:48:54Z

...eBuild/content/test/Microsoft.DotNet.Installer.Tests/Microsoft.DotNet.Installer.Tests.csproj

+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Newtonsoft.Json" />


Given this is a new project, consider taking a dependency on STJ.

Will take a look and update. We might not even need a dependency.

Fixed with 08a4230

MichaelSimons · 2025-02-19T20:00:00Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+
+    private readonly string[] RpmDistroImages =
+    [
+        "mcr.microsoft.com/dotnet/runtime-deps:9.0-azurelinux3.0"


The version should be shared across RPM and Deb to ensure consistency and reduce maintenance.

I am afraid this is going to get stale quickly.

This should be using the 10-preview images out of nightly.

Makes sense - will update.

Fixed with 08a4230

MichaelSimons · 2025-02-19T20:00:38Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+    private const string NetStandard21RpmPackage = @"https://dotnetcli.blob.core.windows.net/dotnet/Runtime/3.1.0/netstandard-targeting-pack-2.1.0-x64.rpm";
+    private const string NetStandard21DebPackage = @"https://dotnetcli.blob.core.windows.net/dotnet/Runtime/3.1.0/netstandard-targeting-pack-2.1.0-x64.deb";
+
+    private enum DistroType


Suggested change

private enum DistroType

private enum PackageType

Will fix - thanks!

Fixed with 08a4230

MichaelSimons · 2025-02-19T20:02:22Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+    [Fact]
+    public void RunScenarioTestsForAllDistros()
+    {
+        if (Config.TestRpmPackages)


Ideally these would be separate testcases. One of the primary reasons for this is that if one fails you would still get a read on the other. Also having explicitly testcases helps make it more obvious what is being tested.

Yes, totally agree. Will do that.

Fixed with 64fa1b8

MichaelSimons · 2025-02-19T20:03:14Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+    private readonly string _tmpDir;
+    private readonly string _contextDir;
+
+    private readonly string[] RpmDistroImages =


Why is this an array? What is the thinking on how this would expand?

We will likely add more images. Having it as array makes the future modification simpler.

I'm changing this to InlineData in a Theory.

Fixed with 64fa1b8

MichaelSimons · 2025-02-19T20:15:31Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+            {
+                output = e.Message;
+            }
+            Assert.Fail($"Build failed: {output}");


How do you know this failed during the build vs run?

We don't - will refactor this.

Fixed with 64fa1b8

MichaelSimons · 2025-02-19T20:18:06Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+        sb.AppendLine("");
+        sb.AppendLine($"ENTRYPOINT [ \"dotnet\", \"{scenarioTestsBinary}\", \"--dotnet-root\", \"/usr/share/dotnet\" ]");
+
+        string dockerfile = Path.Combine(_contextDir, Path.GetRandomFileName());


I feel like debugging would be easier if the filename started with Dockerfile with a random suffix.

Fixed with 08a4230

MichaelSimons · 2025-02-19T20:22:58Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+        if (distroType == DistroType.Rpm)
+        {
+            string? depsId = null;
+            if (baseImage.Contains("opensuse"))


Why is this not hardcoded to azurelinux since it appears to be the only exercisable codepath?

We will likely add more at later time. Having support for other distros is cheap.

MichaelSimons · 2025-02-19T20:24:25Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+    private string GetContentPackage(string prefix, DistroType distroType)
+    {
+        string matchPattern = DistroType.Deb == distroType ? "*.deb" : "*.rpm";
+        string[] rpmFiles = Directory.GetFiles(_contextDir, prefix + matchPattern, SearchOption.AllDirectories)


These aren't necessarily rpm packages right? They could be deb files as well? If so the variable and exception message should be updated.

Fixed with 08a4230

MichaelSimons · 2025-02-19T20:26:16Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+        sb.AppendLine($"COPY packages packages");
+
+        sb.AppendLine("");
+        sb.AppendLine("# Copy RPM packages");


These aren't necessarily RPM packages. There are a few RPM references

Fixed with 08a4230

NikolaMilosavljevic · 2025-02-20T17:21:56Z

With 64fa1b8, I've split the tests and switched to ConditionalTheory. I did have to use repo and tag as separate inline data, as xUnit prints only 50 characters of any inline data string. This can only be tweaked in xUnit v3.

I was getting the following, when using image as inline data:
Passed Microsoft.DotNet.Installer.Tests.LinuxInstallerTests.DebTest(image: "mcr.microsoft.com/dotnet/nightly/runtime-deps:10.0"···) [6 m 10 s]

Splitting into repo and tag produces a nicer output:
Passed Microsoft.DotNet.Installer.Tests.LinuxInstallerTests.DebTest(repo: "mcr.microsoft.com/dotnet/nightly/runtime-deps", tag: "10.0-preview-trixie-slim") [6 m 14 s]

MichaelSimons · 2025-02-20T18:43:49Z

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs

+    }
+
+    [ConditionalTheory(typeof(LinuxInstallerTests), nameof(IncludeRpmTests))]
+    [InlineData("mcr.microsoft.com/dotnet/nightly/runtime-deps", "10.0-preview-azurelinux3.0")]


Consider shared constants here to ensure consistency and reduce maintenance across the Rpm and Deb testcases.

Suggested change

[InlineData("mcr.microsoft.com/dotnet/nightly/runtime-deps", "10.0-preview-azurelinux3.0")]

[InlineData(RuntimeDepsRepo, $"{RuntimeDepsVersion}-azurelinux3.0")]

Yeah, that's good - will fix.

Fixed with 1f69d81

Add installer tests project

366664f

Copilot bot review requested due to automatic review settings February 18, 2025 23:16

NikolaMilosavljevic requested review from a team as code owners February 18, 2025 23:16

dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels Feb 18, 2025

Copilot AI reviewed Feb 18, 2025

View reviewed changes

build-analysis bot mentioned this pull request Feb 19, 2025

restarted. Azure DevOps can't recover from restarts. dotnet/dnceng#3879

Open

3 tasks

jkoritzinsky requested changes Feb 19, 2025

View reviewed changes

Addressing PR comments

054366b

jkoritzinsky approved these changes Feb 19, 2025

View reviewed changes

MichaelSimons reviewed Feb 19, 2025

View reviewed changes

Address more review comments

08a4230

This was referenced Feb 20, 2025

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Open

dotnet-watch tests failing #40006

Open

[dotnet watch] Cannot open project Lib.fsproj because extension fsproj is not associated with a language #46378

Open

Split the tests and use conditional theory

64fa1b8

MichaelSimons reviewed Feb 20, 2025

View reviewed changes

NikolaMilosavljevic added 2 commits February 21, 2025 05:02

Few tweaks

1f69d81

Refactor context initialization

a459cb9

	Assert.Fail("Scenario tests package not found");
	Assert.True(false, "Scenario tests package not found");

	Assert.Fail("Test summary not found");
	Assert.True(false, "Test summary not found");

	[InlineData("mcr.microsoft.com/dotnet/nightly/runtime-deps", "10.0-preview-azurelinux3.0")]
	[InlineData(RuntimeDepsRepo, $"{RuntimeDepsVersion}-azurelinux3.0")]

Add installer tests project #46927

Are you sure you want to change the base?

Add installer tests project #46927

Conversation

NikolaMilosavljevic commented Feb 18, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot AI Feb 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Copilot AI Feb 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NikolaMilosavljevic commented Feb 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment