Make SmokeTests use repo infrastructure #19290

ViktorHofer · 2024-04-03T13:51:34Z

Fixes dotnet/source-build#3615
Fixes dotnet/source-build#4289

This allows the test project to just be invoked with dotnet test or dotnet build /t:Test. Running the UnifiedBuild test projects requires the VMR to first be built to have access to the live built SDK but that's already the case.

Stop depending on environment variables for state instead use RuntimeConfiguration values same as UnifiedBuild test project. Allow facts and theories to be conditioned based on boolean values instead of env vars.
Update the README with the new msbuild property switch names
Delete dead-code and the checked-in .sln file
Update YMLs
Update build scripts to call into the Test target instead.
Use the Microsoft.Build.Traversal SDK inside build.proj
Borrow some of the script changes from Enable scenario test execution in VMR build #19222 to be able to invoke tests to build and test them or just test them.

This allows the test project to just be invoked with `dotnet test` or `dotnet build /t:Test` after the VMR is built. 1. Stop depending on environment variables for state instead use RuntimeConfiguration values same as UnifiedBuild test project. Allow facts and theories to be conditioned based on boolean values instead of env vars. 2. Update the README with the new msbuild property switch names 3. Delete dead-code and the checked-in .sln file

src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/ArtifactsSizeTest.cs

src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/LicenseScanTests.cs

src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/OmniSharpTests.cs

src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/README.md

into RefactorSmokeTests

…ltSdkVersion

ViktorHofer · 2024-04-08T11:04:14Z

Found an AFAICT unintended breaking change in the FileSytemGlobbing API between 8.0.0 and 9.0 Preview 1 and filed an issue for it: dotnet/runtime#100762

I worked around it by downgrading the packages to 8.0.0.

cc @jozkee @jeffhandley

ViktorHofer · 2024-04-08T12:07:38Z

Validation builds:

dotnet-source-build build: https://dnceng.visualstudio.com/internal/_build/results?buildId=2425052&view=results (the CentOS online leg is configured to run the poison tests in my branch)
source-build-license-scan build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2424915&view=results
source-build-sdk-diff-tests build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2424979&view=results

Summary:

The dotnet-source-build internal full build has a lot of red but the red is identical with the latest scheduled/rolling build. The legs that pass run the smoke tests as expected and they succeed.
The license scan tests run as expected and succeed.
The SDK diff tests don't run because it can't find a corresponding installer build. That seems to be related to the manual vmr-sync. Given the already exhaustive testing that I performed, I would say let's merge this in and I will monitor the next build and respond to any potential regressions. Note that I locally ran these tests and they succeeded.
The poison tests run as expected and report the following (AFAIK known) poison leaks:

<PrebuiltLeakReport>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/DotnetTools/dotnet-format/Microsoft.CodeAnalysis.AnalyzerUtilities.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/DotnetTools/dotnet-watch/x.y.z/tools/netx.y/any/Microsoft.CodeAnalysis.AnalyzerUtilities.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/cs/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/de/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/es/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/fr/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/it/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/ja/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/ko/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/Microsoft.CodeAnalysis.CSharp.NetAnalyzers.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/Microsoft.CodeAnalysis.NetAnalyzers.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/Microsoft.CodeAnalysis.VisualBasic.NetAnalyzers.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/pl/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/pt-BR/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/ru/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/tr/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/zh-Hans/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
  <File Path="artifacts/assets/Release/dotnet-sdk-x.y.z/sdk/x.y.z/Sdks/Microsoft.NET.Sdk/analyzers/zh-Hant/Microsoft.CodeAnalysis.NetAnalyzers.resources.dll">
    <Type>AssemblyAttribute</Type>
  </File>
</PrebuiltLeakReport>

mthalman

In the future, consider breaking up changes like this into separate PRs to allow for more focused reviews. I feel like this could have easily been 3 separate PRs or even more.

src/SourceBuild/content/Directory.Build.targets

src/SourceBuild/content/build.sh

mthalman · 2024-04-08T16:05:37Z

src/SourceBuild/content/repo-projects/nuget-client.proj

@@ -15,6 +15,7 @@
  </ItemGroup>

  <ItemGroup>
+    <EnvironmentVariables Include="NUGET_PACKAGES=$(NuGetPackageRoot)" />


What does this have to do with test infrastructure?

Before, we globally set NUGET_PACKAGES but we don't do that anymore so that we can have a separate nuget package cache for the VMR tests. Nuget-client is the only repo that doesn't respect the NUGET_PACKAGES env var that is provided by Arcade because they don't use Arcade: https://github.com/dotnet/arcade/blob/f54c598d137ab88bd473ea670b3c751be0540244/src/Microsoft.DotNet.Arcade.Sdk/tools/SourceBuild/SourceBuildArcadeBuild.targets#L90

I filed dotnet/source-build#4293 which is relevant here. nuget-client incorrectly uses the VMR package cache (already in main).

...ourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/SourceBuiltArtifactsTests.cs

ellahathaway · 2024-04-08T16:30:16Z

In your PR description, you mention that we can use dotnet test to invoke the tests. I tried just now to invoke the tests via .dotnet/dotnet test src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/ from the installer repo, but it failed with this error:

/workspaces/installer/src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/DotNetHelper.cs(38): error VSTEST1: (Microsoft.DotNet.SourceBuild.SmokeTests.SourcelinkTests.VerifySourcelinks) Microsoft.DotNet.SourceBuild.SmokeTests.DotNetHelper..ctor(ITestOutputHelper outputHelper) System.InvalidOperationException : Tarball path '' specified in  does not exist.

I was under the impression that the Sdk tarball path had a default (added by your changes). Can you explain why that default was not set properly when I invoked the tests in this way? Is it because the default is looking for a built SDK in the artifacts? If that's the case, I'd rather this not be the failing exception. Rather, the failure should happen when the tarball path variable is originally set in my opinion.

ViktorHofer · 2024-04-08T16:34:50Z

I was under the impression that the Sdk tarball path had a default (added by your changes). Can you explain why that default was not set properly when I invoked the tests in this way?

Yes, when you build the repo from within the VMR, the Sdk tarball has its default value. The resolution logic depends on this target:

installer/src/SourceBuild/content/Directory.Build.targets

Line 5 in 8db3f66

which relies on installer produced packages. If you want to execute a test that depends on the live built SDK, you either need to build the VMR first or supply the SDK via the /p:SdkTarballPath property. Please let me know if this scenario worked before but is now regressed. I don't think that's the case.

What I meant is that you can now test the smoke tests project directly with dotnet test but you need to filter down the test that you want to execute and/or supply all the required inputs.

src/SourceBuild/content/build.sh

src/SourceBuild/content/eng/Versions.props

src/SourceBuild/content/test/TestUtilities/ConditionalDiscovererException.cs

.../test/Microsoft.DotNet.SourceBuild.SmokeTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj

ViktorHofer · 2024-04-08T16:39:44Z

In the future, consider breaking up changes like this into separate PRs to allow for more focused reviews. I feel like this could have easily been 3 separate PRs or even more.

When you look at the commits you will noticed that I originally started just with the bare minimum number of changes and then only added to those because the build required those. This organically grew from ~20 to 50 file changes. If you feel that separating these changes into individual PRs is a good investment of time then I can try that but it will be hard because many of these are intertangled.

mthalman · 2024-04-08T17:33:15Z

In the future, consider breaking up changes like this into separate PRs to allow for more focused reviews. I feel like this could have easily been 3 separate PRs or even more.

When you look at the commits you will noticed that I originally started just with the bare minimum number of changes and then only added to those because the build required those. This organically grew from ~20 to 50 file changes. If you feel that separating these changes into individual PRs is a good investment of time then I can try that but it will be hard because many of these are intertangled.

No, not necessary to break up this PR at this point.

.../test/Microsoft.DotNet.SourceBuild.SmokeTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj

MichaelSimons · 2024-04-08T20:58:05Z

Looks like there are still compilation errors:

/vmr/test/Microsoft.DotNet.SourceBuild.SmokeTests/OmniSharpTests.cs(30,6): error CS0246: The type or namespace name 'ConditionalTheoryAttributeAttribute' could not be found (are you missing a using directive or an assembly reference?) [/vmr/test/Microsoft.DotNet.SourceBuild.SmokeTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj]
/vmr/test/Microsoft.DotNet.SourceBuild.SmokeTests/OmniSharpTests.cs(30,6): error CS0246: The type or namespace name 'ConditionalTheoryAttribute' could not be found (are you missing a using directive or an assembly reference?) [/vmr/test/Microsoft.DotNet.SourceBuild.SmokeTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj]

ViktorHofer · 2024-04-08T21:14:26Z

Ooops, forgot to build everything. That's now fixed.

ViktorHofer mentioned this pull request Apr 3, 2024

VMR smoke test project hardcodes package versions and TFM dotnet/source-build#4289

Closed

ViktorHofer force-pushed the RefactorSmokeTests branch from eaeebd9 to c3d4094 Compare April 3, 2024 14:00

ViktorHofer force-pushed the RefactorSmokeTests branch from c3d4094 to a3a75bd Compare April 3, 2024 15:05

ellahathaway reviewed Apr 3, 2024

View reviewed changes

ViktorHofer added 25 commits April 3, 2024 17:44

PR feedback

15501b2

Fixes

29a19a4

More PR feedback

1b35086

Better calculate SdkTarballPath and SourceBuiltSdkVersion

1f6c7e9

Clean-up

a8ae19c

Merge branch 'main' into RefactorSmokeTests

1b96515

Add restore flag

45cdca3

Merge branch 'RefactorSmokeTests' of https://github.com/dotnet/installer

c1b59a7

into RefactorSmokeTests

Fix bugs

812c3d9

Fix SDK version

025bfe6

YML updates

b1b8fc9

nit

9552f76

Merge branch 'main' into RefactorSmokeTests

6263ca4

Update Config.cs

ba97bfa

Update build.ps1

0169f88

More fixes and clean-up

0d06bd5

More fixes and clean-up

594b289

Updates to smoketest dependency and revert most of DetermineSourceBui…

458085f

…ltSdkVersion

Fix missing Project tag

53c3f02

Revert the non desirable changes in instaler.proj

d4727b9

Missed one

33709bd

One more

4e1ecae

Improve condition in smoketests

0709b16

Fix windows script

d8de1bd

Fix missing \

a509077

ViktorHofer added 4 commits April 7, 2024 18:29

Upload artifacts also in failure case

74c74a3

Throw in case of not found tar artifact

e83b6b8

Downgrade version to 8.0.0 to avoid breaking change

95c0a32

Merge branch 'main' into RefactorSmokeTests

6eedd6b

Include trx files in artifacts log publish

911b11c

ViktorHofer enabled auto-merge (squash) April 8, 2024 14:11

mthalman disabled auto-merge April 8, 2024 16:06

Update vmr-build.yml

3a9a7ba

mthalman reviewed Apr 8, 2024

View reviewed changes

Update SourceBuiltArtifactsTests.cs

ed0a320

MichaelSimons reviewed Apr 8, 2024

View reviewed changes

ViktorHofer added 3 commits April 8, 2024 18:50

Remove --test-no-build/-testNoBuild switch

830d0ef

Update xunit namespace to TestUtilities

3380d9f

Fix build error in TestUtilities

2f75f98

ViktorHofer added 2 commits April 8, 2024 20:31

Fix compliation error and revert artifacts change

0c6058e

revert unnecessary changes in SourceBuiltArtifactsTests

9976f8a

ellahathaway reviewed Apr 8, 2024

View reviewed changes

.../test/Microsoft.DotNet.SourceBuild.SmokeTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj Show resolved Hide resolved

MichaelSimons approved these changes Apr 8, 2024

View reviewed changes

ViktorHofer merged commit be917a9 into main Apr 9, 2024
22 checks passed

ViktorHofer deleted the RefactorSmokeTests branch April 9, 2024 05:00

mthalman mentioned this pull request Apr 10, 2024

Enable scenario test execution in VMR build #19222

Merged

MichaelSimons mentioned this pull request Aug 2, 2024

Place smoke test build output in the tarball's artifacts directory dotnet/source-build#2793

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make SmokeTests use repo infrastructure #19290

Make SmokeTests use repo infrastructure #19290

ViktorHofer commented Apr 3, 2024 •

edited

Loading

ViktorHofer commented Apr 8, 2024 •

edited

Loading

ViktorHofer commented Apr 8, 2024 •

edited

Loading

mthalman left a comment

mthalman Apr 8, 2024

ViktorHofer Apr 8, 2024

ellahathaway commented Apr 8, 2024 •

edited

Loading

ViktorHofer commented Apr 8, 2024 •

edited

Loading

ViktorHofer commented Apr 8, 2024

mthalman commented Apr 8, 2024

MichaelSimons commented Apr 8, 2024

ViktorHofer commented Apr 8, 2024

Make SmokeTests use repo infrastructure #19290

Make SmokeTests use repo infrastructure #19290

Conversation

ViktorHofer commented Apr 3, 2024 • edited Loading

ViktorHofer commented Apr 8, 2024 • edited Loading

ViktorHofer commented Apr 8, 2024 • edited Loading

mthalman left a comment

Choose a reason for hiding this comment

mthalman Apr 8, 2024

Choose a reason for hiding this comment

ViktorHofer Apr 8, 2024

Choose a reason for hiding this comment

ellahathaway commented Apr 8, 2024 • edited Loading

ViktorHofer commented Apr 8, 2024 • edited Loading

ViktorHofer commented Apr 8, 2024

mthalman commented Apr 8, 2024

MichaelSimons commented Apr 8, 2024

ViktorHofer commented Apr 8, 2024

ViktorHofer commented Apr 3, 2024 •

edited

Loading

ViktorHofer commented Apr 8, 2024 •

edited

Loading

ViktorHofer commented Apr 8, 2024 •

edited

Loading

ellahathaway commented Apr 8, 2024 •

edited

Loading

ViktorHofer commented Apr 8, 2024 •

edited

Loading