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.

Why not refactor at that time? I don't like the thought of adding code that is not certain to be used, it is a form of technical debt. We don't have runtime-deps images for these distros. What is the ROI of adding them?

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]

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

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

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

+        string[] files = Directory.GetFiles(_contextDir, prefix + matchPattern, SearchOption.AllDirectories)
+            .Where(p => !Path.GetFileName(p).Contains("dotnet-runtime-deps-"))
+            .ToArray();
+        if (files.Length == 0)


Curious if multiple are expected?

MichaelSimons · 2025-02-24T20:40:42Z

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

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


Why not refactor at that time? I don't like the thought of adding code that is not certain to be used, it is a form of technical debt. We don't have runtime-deps images for these distros. What is the ROI of adding them?

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

MichaelSimons · 2025-02-24T20:46:46Z

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

+        sb.AppendLine("# Install the installer packages and Microsoft.DotNet.ScenarioTests.SdkTemplateTests tool");
+        sb.Append("RUN");
+
+        // TODO: remove --force-all when aspnet package versioning issue have been resolved - https://github.com/dotnet/source-build/issues/4895


It looks like this has been resolved. Can the workaround be removed now?

Yes, this should be in VMR now - will confirm and update the code.

We still need this workaround due to Debain DEPs package that requires libgssapi-krb5-2. The package is missing on mcr.microsoft.com/dotnet/nightly/runtime-deps:10.0-preview-trixie-slim

dotnet/dotnet-docker#6271

MichaelSimons · 2025-02-24T20:48:46Z

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

+
+        // Set entry point
+        sb.AppendLine("");
+        sb.AppendLine($"ENTRYPOINT [ \"dotnet\", \"{scenarioTestsBinary}\", \"--dotnet-root\", \"/usr/share/dotnet\" ]");


Curious why the entrypoint approach was used versus specifying via Docker run?

No particular reason. Does docker run approach provide any additional benefits? I understand that entrypoint can be overridden when running this image, i.e. for debugging.

I was mainly asking from a readability perspective of the DistroTest method - it is not obvious what tests are actually being run.

Makes sense. Yeah, you're right. The way it is today, this command is hidden in dockerfile generation method. I'll update this in a follow up PR.

I have a fix for this and will push the change soon.

Fixed with 004efb2

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

…LinuxInstallerTests.cs Co-authored-by: Michael Simons <[email protected]>

NikolaMilosavljevic · 2025-02-25T21:31:36Z

I need help merging this - it is hitting the unrelated test issues that are being worked on atm. @MichaelSimons

MichaelSimons · 2025-02-26T14:24:39Z

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 const string RuntimeDepsRepo = "mcr.microsoft.com/dotnet/nightly/runtime-deps";


Now that Preview 1 was released - this should be switched to "mcr.microsoft.com/dotnet/runtime-deps". @NikolaMilosavljevic, can you include that change as part of hooking these tests up to a pipeline? TIA

ViktorHofer · 2025-02-26T14:43:43Z

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

+            // Find the scenario-tests package and unpack it to the context dir, subfolder "scenario-tests"
+            string? scenarioTestsPackage = Directory.GetFiles(nugetPackagesDir, "Microsoft.DotNet.ScenarioTests.SdkTemplateTests*.nupkg", SearchOption.AllDirectories).FirstOrDefault();


I'm struggling to follow the execution flow here. Where is that package coming from?

Does this project have a dependency on the scenario-tests.proj build?

Microsoft.DotNet.ScenarioTests.SdkTemplateTests*.nupkg package is a product of the scenario-tests repo. I did mention this yesterday at the meeting as a need to have this repo built separately from testing it - as it is today.

However, the plan is for installer tests to run during VMR Validation stage, and download all packages produced during VMR build. Scenario-tests package should be built, if not, that should be changed.

I will be testing the second part of installer-tests work soon, the pipeline hook-up.

If the abovementioned plan is not possible, I'd add a dependency on scenario-tests repo project and have it built as part of installer testing.

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

src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/LinuxInstallerTests.cs Outdated Show resolved Hide resolved

NikolaMilosavljevic added 2 commits February 21, 2025 05:02

Few tweaks

1f69d81

Refactor context initialization

a459cb9

MichaelSimons reviewed Feb 24, 2025

View reviewed changes

NikolaMilosavljevic and others added 2 commits February 24, 2025 13:31

Update src/SourceBuild/content/test/Microsoft.DotNet.Installer.Tests/…

6ac9570

…LinuxInstallerTests.cs Co-authored-by: Michael Simons <[email protected]>

Address few more comments

7dd918f

NikolaMilosavljevic mentioned this pull request Feb 25, 2025

Trixie image does not contain the required package - libgssapi-krb5-2 dotnet/dotnet-docker#6271

Open

Specify test command in docker run command

004efb2

MichaelSimons approved these changes Feb 25, 2025

View reviewed changes

MichaelSimons merged commit bc742b0 into dotnet:main Feb 25, 2025
34 of 40 checks passed

MichaelSimons reviewed Feb 26, 2025

View reviewed changes

ViktorHofer reviewed Feb 26, 2025

View reviewed changes

	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");

		// Find the scenario-tests package and unpack it to the context dir, subfolder "scenario-tests"
		string? scenarioTestsPackage = Directory.GetFiles(nugetPackagesDir, "Microsoft.DotNet.ScenarioTests.SdkTemplateTests*.nupkg", SearchOption.AllDirectories).FirstOrDefault();

Add installer tests project #46927

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

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

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 25, 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