Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
208 changes: 208 additions & 0 deletions .github/workflows/tests-runner.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
# Executes all the tests on all the platforms
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this replace tests.yml+run-tests.yml? How is this being used?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's the plan to use this instead of the other existing workflows.
Right now, this is to be kicked off manually, since this runsheet isn't running the template tests (per your request). We can enable running this alongside with the existing workflow (to test the templates) but we'll need to remove all other tests from test.yml. Essentially, this is the follow up work.

Copy link
Copy Markdown
Member

@radical radical Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please help me understand why we are repeating the stuff from tests.yml, and run-tests.yml here? Can they not be re-used with some relevant command line changes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is supposed to replace tests.yml+run-tests.yml, however we can't do it right now until the Templates tests are also moved to the runsheet.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we be able to re-use run-tests.yml as-is, instead of splitting and duplicating it here, and then debugging the new setup again? That way we keep using the existing stuff and replace the test generation stuff with the runsheets.

Copy link
Copy Markdown
Contributor Author

@RussKie RussKie Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing implementation does a lot more, and I don't want to spend time trying to selectively clean or refactor it to ensure the Template tests continue to run.
I need this change so we can verify it and continue to build on top of it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It creates extra work to review the changes here and in following PRs, which are partially repeating the existing working implementation without being sure that it works. I don't understand the reason for throwing out the working thing.

I'll approve here, and we can discuss when you enable this, I guess.

name: Tests

on:
workflow_dispatch:

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
# Duplicated jobs so their dependencies are not blocked on both the
# setup jobs

# Generates a runsheet for all the tests in the solution that do not require
# NuGet packages to be built.
# The runsheet generation is expected to be fast.
generate_tests_matrix:
name: Generate test runsheet
runs-on: windows-latest
outputs:
runsheet: ${{ steps.generate_tests_matrix.outputs.runsheet }}
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2

# In order to build the solution, we need to install the SDK and the toolsets first
# as defined in global.json. For this, create a temporary project file and run the
# build command with the -restore option. This will install the SDK and toolsets.
#
# We don't want to run 'build.cmd -restore' as it will also restore all the packages,
# which takes a long time and is not needed for this job.
- name: Install toolsets
shell: pwsh
run: |
mkdir ./artifacts/tmp -force | Out-Null
'<Project />' | Out-File -FilePath ./artifacts/tmp/install-toolset.proj -Encoding utf8
./build.cmd -restore -projects ./artifacts/tmp/install-toolset.proj

- name: Generate test runsheet
id: generate_tests_matrix
shell: pwsh
run: |
./build.cmd -test /p:TestRunnerName=TestRunsheetBuilder -bl -c Release

- name: Upload logs, and test results
if: ${{ always() }}
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
with:
name: runsheet-logs
path: |
${{ github.workspace }}/artifacts/log/*/*.binlog
${{ github.workspace }}/artifacts/log/*/TestLogs/**
${{ github.workspace }}/artifacts/tmp/*/combined_runsheet.json
retention-days: 3

# Generates a runsheet for all the tests in the solution that DO require
# NuGet packages to be built.
# The runsheet generation is expected to be slow as we need to restore and build
# the whole solution and publish all the packages.
generate_e2e_matrix:
name: Generate E2E test runsheet
runs-on: windows-latest
outputs:
runsheet: ${{ steps.generate_e2e_matrix.outputs.runsheet }}
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2

- name: Build with packages
run: |
./build.cmd -restore -build -pack -c Release -ci -bl /p:InstallBrowsersForPlaywright=false /p:SkipTestProjects=true /p:CI=false

- name: Generate test runsheet
id: generate_e2e_matrix
run: |
./build.cmd -test /p:TestRunnerName=TestRunsheetBuilder /p:FullE2e=true -bl -c Release

- name: Upload built NuGets
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
with:
name: built-nugets
path: artifacts/packages
retention-days: 3

- name: Upload logs, and test results
if: ${{ always() }}
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
with:
name: runsheet-e2e-logs
path: |
${{ github.workspace }}/artifacts/log/*/*.binlog
${{ github.workspace }}/artifacts/log/*/TestLogs/**
${{ github.workspace }}/artifacts/tmp/*/combined_runsheet.json
retention-days: 3

run_tests:
name: Test
needs: generate_tests_matrix
strategy:
fail-fast: false
matrix:
tests: ${{ fromJson(needs.generate_tests_matrix.outputs.runsheet) }}

runs-on: ${{ matrix.tests.os }} # Use the OS from the matrix

steps:
- name: Trust HTTPS development certificate
if: runner.os == 'Linux'
run: dotnet dev-certs https --trust

- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2

- name: Verify Docker is running
# nested docker containers not supported on windows
if: runner.os == 'Linux'
run: docker info

- name: Install Azure Functions Core Tools
if: runner.os == 'Linux'
run: |
sudo apt-get update
sudo apt-get install -y azure-functions-core-tools-4

- name: Test ${{ matrix.tests.project }}
run: |
${{ matrix.tests.command }}

- name: Upload test results
if: always()
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
with:
name: ${{ matrix.tests.project }}-${{ matrix.tests.os }}-logs
path: |
${{ github.workspace }}/artifacts/TestResults/*/*.trx
${{ github.workspace }}/artifacts/log/*/TestLogs/**
retention-days: 30

- name: Upload logs
if: failure()
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
with:
name: ${{ matrix.tests.project }}-${{ matrix.tests.os }}-binlogs
path: |
${{ github.workspace }}/artifacts/log/*/*.binlog
retention-days: 3

run_e2e_tests:
name: E2ETest
needs: generate_e2e_matrix
strategy:
fail-fast: false
matrix:
tests: ${{ fromJson(needs.generate_e2e_matrix.outputs.runsheet) }}

runs-on: ${{ matrix.tests.os }} # Use the OS from the matrix

steps:
- name: Trust HTTPS development certificate
if: runner.os == 'Linux'
run: dotnet dev-certs https --trust

- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2

- name: Download built NuGets
uses: actions/download-artifact@cc203385981b70ca67e1cc392babf9cc229d5806 # v4.1.9
with:
pattern: built-nugets
path: ${{ github.workspace }}/artifacts/packages

- name: Copy NuGets to the correct location
shell: pwsh
run:
Move-Item -Path "${{ github.workspace }}/artifacts/packages/built-nugets/Release" -Destination "${{ github.workspace }}/artifacts/packages"

- name: Verify Docker is running
# nested docker containers not supported on windows
if: runner.os == 'Linux'
run: docker info

- name: Install Azure Functions Core Tools
if: runner.os == 'Linux'
run: |
sudo apt-get update
sudo apt-get install -y azure-functions-core-tools-4

- name: Test ${{ matrix.tests.project }}
env:
BUILT_NUGETS_PATH: ${{ github.workspace }}/artifacts/packages/Release/Shipping
run: |
${{ matrix.tests.command }}

- name: Upload test results
if: always()
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
with:
name: ${{ matrix.tests.project }}-${{ matrix.tests.os }}-logs
path: |
${{ github.workspace }}/artifacts/TestResults/*/*.trx
${{ github.workspace }}/artifacts/log/*/TestLogs/**
retention-days: 30

- name: Upload logs
if: failure()
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
with:
name: ${{ matrix.tests.project }}-${{ matrix.tests.os }}-binlogs
path: |
${{ github.workspace }}/artifacts/log/*/*.binlog
retention-days: 3
6 changes: 4 additions & 2 deletions eng/AfterSolutionBuild.targets
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
```

-->
<Target Name="_GenerateTestMatrix" BeforeTargets="Test" Condition=" '$(TestRunnerName)' == 'QuarantinedTestRunsheetBuilder' ">
<Target Name="_GenerateTestMatrix" BeforeTargets="Test" Condition=" '$(TestRunnerName)' == 'TestRunsheetBuilder' or '$(TestRunnerName)' == 'QuarantinedTestRunsheetBuilder' ">
<PropertyGroup>
<_CombinedRunsheetFile>$(ArtifactsTmpDir)/combined_runsheet.json</_CombinedRunsheetFile>
<_Command>
Expand All @@ -65,7 +65,9 @@
$combined += @($content)
}
}
$jsonString = ($combined | ConvertTo-Json -Depth 10 -Compress)

# See https://collectingwisdom.com/powershell-convertto-json-single-item-array/
$jsonString = (ConvertTo-Json $combined -Depth 10 -Compress)
$jsonString | Set-Content '$(_CombinedRunsheetFile)';

# determine if the script is running in a GitHub Actions environment
Expand Down
109 changes: 109 additions & 0 deletions eng/TestRunsheetBuilder/TestRunsheetBuilder.targets
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
<Project>
<!--

This file is used to generate a list of tests to run on GitHub Actions.

- For projects not requiring packages:
.\build.cmd -test /p:TestRunnerName=TestRunsheetBuilder [/bl /p:GITHUB_ACTIONS=true]

- For projects requiring packages (e.g., E2E, templates and playground):
.\build.cmd -test /p:TestRunnerName=TestRunsheetBuilder /p:FullE2e=true [/bl /p:GITHUB_ACTIONS=true]

For the large part this is a copy of the Arcade SDK's implementations:
- https://github.com/dotnet/arcade/blob/b888df17/src/Microsoft.DotNet.Arcade.Sdk/tools/XUnit/XUnit.Runner.targets
- https://github.com/dotnet/arcade/blob/b888df17/src/Microsoft.DotNet.Arcade.Sdk/tools/VSTest.targets
-->

<Target Name="RunTests"
Outputs="%(TestToRun.ResultsStdOutPath)"
Condition=" '$(SkipTests)' != 'true' and '$(IsGitHubActionsRunner)' == 'true' and '$(RunOnGithubActions)' == 'true' ">

<PropertyGroup>
<!--

The runsheet is created in two scenarios:
- For projects requiring packages, only if full end-to-end tests (`FullE2e`) are enabled.
- For projects not requiring packages, only if full end-to-end tests (`FullE2e`) are disabled.

This logic ensures runsheets are generated appropriately based on the project's characteristics and the testing scenario.

FIXME: Aspire.Playground.Tests are currently not running in "out of repo" mode.
FIXME: Aspire.Templates.Tests are currently left alone.

-->
<_RequiresPackages Condition=" '$(MSBuildProjectName)' == 'Aspire.EndToEnd.Tests' ">true</_RequiresPackages>

<_CreateRunsheet>false</_CreateRunsheet>
<_CreateRunsheet Condition=" '$(_RequiresPackages)' == 'true' and '$(FullE2e)' == 'true' ">true</_CreateRunsheet>
<_CreateRunsheet Condition=" '$(_RequiresPackages)' != 'true' and '$(FullE2e)' != 'true' ">true</_CreateRunsheet>

<_CreateRunsheet Condition=" '$(MSBuildProjectName)' == 'Aspire.Templates.Tests' ">false</_CreateRunsheet>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@radical this is where the Templates are locked out atm; it should be a matter of removing this line.

</PropertyGroup>

<PropertyGroup>
<!--
We do not care whether the project is multi-targeting, we're only generating a command to kick off the testing sequence,
which in turn will run the tests for all the target frameworks.

So, instead of using "%(TestToRun.ResultsFilePathWithoutExtension)" (which looks something like "Aspire.Cli.Tests_net8.0_x64")
we use the project name (which looks something like "Aspire.Cli.Tests").
-->
<_TestRunsheet>$(MSBuildProjectName)</_TestRunsheet>
<_TestRunsheetFileNameWindows>$(ArtifactsTmpDir)\$(_TestRunsheet).win.runsheet.json</_TestRunsheetFileNameWindows>
<_TestRunsheetFileNameLinux>$(ArtifactsTmpDir)\$(_TestRunsheet).linux.runsheet.json</_TestRunsheetFileNameLinux>

<_TestBinLog>$([MSBuild]::NormalizePath($(ArtifactsLogDir), '$(_TestRunsheet).binlog'))</_TestBinLog>

<_RelativeTestProjectPath>$([System.String]::Copy('$(MSBuildProjectFullPath)').Replace('$(RepoRoot)', '%24(pwd)/'))</_RelativeTestProjectPath>
<_RelativeTestBinLog>$([System.String]::Copy('$(_TestBinLog)').Replace('$(RepoRoot)', '%24(pwd)/'))</_RelativeTestBinLog>

<_TestRunnerWindows>./eng/build.ps1</_TestRunnerWindows>
<_TestRunnerLinux>./eng/build.sh</_TestRunnerLinux>
<_TestCommand>-restore -build -test -projects &quot;$(_RelativeTestProjectPath)&quot; /bl:&quot;$(_RelativeTestBinLog)&quot; -c $(Configuration) -ci /p:CI=false</_TestCommand>

<!-- Tests requiring packages must be run in the "out of repo" mode -->
<_TestCommand Condition=" '$(_RequiresPackages)' == 'true' ">$(_TestCommand) /p:TestsRunningOutsideOfRepo=true</_TestCommand>

<!-- Replace \ with /, and then escape " with \", so we have a compliant JSON -->
<_TestCommand>$([System.String]::Copy($(_TestCommand)).Replace("\", "/").Replace('&quot;', '\&quot;'))</_TestCommand>

<_TestRunsheetWindows>{ "project": "$(_TestRunsheet)", "os": "windows-latest", "command": "./eng/build.ps1 $(_TestCommand)" }</_TestRunsheetWindows>
<_TestRunsheetLinux>{ "project": "$(_TestRunsheet)", "os": "ubuntu-latest", "command": "./eng/build.sh $(_TestCommand)" }</_TestRunsheetLinux>
</PropertyGroup>

<ItemGroup>
<_OutputFiles Include="$(_TestRunsheetFileNameWindows)" />
<_OutputFiles Include="$(_TestRunsheetFileNameLinux)" />
</ItemGroup>

<MakeDir Directories="@(_OutputFiles->'%(RootDir)%(Directory)')"/>
<Delete Files="@(_OutputFiles)" />

<WriteLinesToFile
Condition=" '$(RunOnGithubActionsWindows)' == 'true' and '$(_CreateRunsheet)' == 'true' "
File="$(_TestRunsheetFileNameWindows)"
Lines="$(_TestRunsheetWindows)"
Overwrite="true"
WriteOnlyWhenDifferent="true" />
<WriteLinesToFile
Condition=" '$(RunOnGithubActionsLinux)' == 'true' and '$(_CreateRunsheet)' == 'true' "
File="$(_TestRunsheetFileNameLinux)"
Lines="$(_TestRunsheetLinux)"
Overwrite="true"
WriteOnlyWhenDifferent="true" />

<!--
On Linux there's a bug in MSBuild, which "normalises" all slashes (see https://github.com/dotnet/msbuild/issues/3468).
This is a workaround to replace `/"` with the required `\"`.
-->
<Exec Command="pwsh -Command &quot;(Get-Content -Path '$(_TestRunsheetFileNameWindows)') -replace '/\&quot;', '\\\&quot;' | Set-Content -Path '$(_TestRunsheetFileNameWindows)'&quot; "
Condition=" Exists('$(_TestRunsheetFileNameWindows)') and '$(BuildOs)' != 'windows' " />
<Exec Command="pwsh -Command &quot;(Get-Content -Path '$(_TestRunsheetFileNameLinux)') -replace '/\&quot;', '\\\&quot;' | Set-Content -Path '$(_TestRunsheetFileNameLinux)'&quot; "
Condition=" Exists('$(_TestRunsheetFileNameLinux)') and '$(BuildOs)' != 'windows' " />

<!--
The final piece of the puzzle is in eng/AfterSolutionBuild.targets, where we combine the runsheets from all the test projects into a single runsheet.
-->
</Target>

</Project>
3 changes: 1 addition & 2 deletions eng/Testing.targets
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@
<SkipTests>true</SkipTests>

<!-- Only run tests if the build is running on GitHub Actions -->
<SkipTests Condition=" '$(IsGitHubActionsRunner)' == 'true' and '$(BuildOs)' == 'windows' and '$(RunOnGithubActionsWindows)' == 'true' ">false</SkipTests>
<SkipTests Condition=" '$(IsGitHubActionsRunner)' == 'true' and '$(BuildOs)' != 'windows' and '$(RunOnGithubActionsLinux)' == 'true' ">false</SkipTests>
<SkipTests Condition=" '$(IsGitHubActionsRunner)' == 'true' and '$(RunOnGithubActions)' == 'true' ">false</SkipTests>

<!-- Only run tests if the build is running on Helix infra -->
<SkipTests Condition=" '$(IsAzdoHelixRunner)' == 'true' and '$(RunOnAzdoHelix)' == 'true' ">false</SkipTests>
Expand Down
8 changes: 8 additions & 0 deletions eng/Xunit3/Microsoft.Testing.Platform.targets
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@
<_ResultsFileToDisplay Condition="!Exists('$(_ResultsFileToDisplay)')">%(TestToRun.ResultsStdOutPath)</_ResultsFileToDisplay>
</PropertyGroup>

<!-- Generate a test report -->
<Exec Command="pwsh -command &quot;$(RepoRoot)eng/scripts/gha-testreport.ps1 -TestResultsFolder '$(_TestResultDirectory)' -TestSummaryOutputFolder '$(TestResultsLogDir)'&quot;"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not simply implement stuff using MTP extensibility?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an interesting idea, how would you approach it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RussKie You'd implement an extension as both IDataConsumer and ITestSessionLifetimeHandler. It will be consume TestNodeUpdateMessages. The test node will have TimingProperty which the duration, and will have TestNodeStateProperty indicating test status (pass, fail, timeout, skipped, cancelled).

While consuming, you'll keep the data you need stored, then in ITestSessionLifetimeHandler.OnTestSessionFinishingAsync you can print the data to OutputDevice (and disable Arcade's redirection), or maybe writing to GITHUB_STEP_SUMMARY can be a better idea anyways?

Condition=" '$(IsGitHubActionsRunner)' == 'true' "
WorkingDirectory="$(RepoRoot)"
IgnoreExitCode="true"
ContinueOnError="WarnAndContinue"
/>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This trick allows us generating a report as part of a test run:
image

However, I totally realise this may not stand the test of time (considering #8802) if we can't figure out how to bake this into the Arcade SDK.


<!--
Ideally we would set ContinueOnError="ErrorAndContinue" so that when a test fails in multi-targeted test project
we'll still run tests for all target frameworks. ErrorAndContinue doesn't work well on Linux though: https://github.com/Microsoft/msbuild/issues/3961.
Expand Down
Loading