[CI] Enable Workload tests for PR validation #7939

radical · 2025-03-06T20:19:56Z

Extract the existing job to enumerate the test class names to a custom action, so that it can be re-used
Split the jobs so linux, and windows ones can run independently. And can have separate list of tests.
Add a new workflow for running Workload tests, with a corresponding new action enumerate-template-tests
Disable tests on windows/CI that need to trust the SSL certificate
Also, add an artifact with all the test results for Integration, and Template workflows.

With this PR the setup jobs run independently for each OS, thus allowing their test jobs to start running immediately. This helps because Windows job can be a little slower, and doesn't block the Linux test jobs.

Fixes #7583 .

davidfowl · 2025-03-07T22:36:36Z

Can you rename these tests while you’re at it

.. and use independent jobs to get list of tests for Windows, and Linux.

.. and disable tests that need that, on windows/ci.

radical · 2025-03-11T04:33:14Z

Can you rename these tests while you’re at it

I'll have a separate PR for that. There are a whole bunch of places with Workload in the name.

Copilot

PR Overview

This PR enables Workload tests for PR validation by adding SSL certificate requirements to tests and updating the CI workflows and test project configurations. The key changes include:

New SSL certificate attribute and trait discoverer to gate tests on proper environment support.
Updates to composite GitHub actions and workflows to generate test matrices for both template and general tests.
Modifications to several test projects, including renaming a base test class and adding the new SSL requirement attribute.

Reviewed Changes

File	Description
tests/Aspire.Components.Common.Tests/RequiresSSLCertificateAttribute.cs	Adds attribute to require SSL certificate for tests.
tests/Aspire.Components.Common.Tests/RequiresSSLCertificateDiscoverer.cs	Implements trait discovery based on SSL certificate support.
.github/actions/enumerate-template-tests/action.yml	Defines composite action for enumerating workload tests.
.github/actions/enumerate-tests/action.yml	Defines composite action for enumerating test projects.
.github/workflows/template-tests.yml & .github/workflows/run-tests.yml	Updates workflows to incorporate test matrix generation and execution.
tests/Aspire.Workload.Tests/StarterTemplateProjectNamesTests.cs	Refactors the test class to be abstract and updates test data generation.
tests/Aspire.Workload.Tests/StarterTemplateWithRedisCacheTests.cs & related PreviousTFM file	Adds SSL certificate requirement; note potential naming duplication.

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

tests/Aspire.Workload.Tests/StarterTemplateWithRedisCacheTests.cs:9

[nitpick] The class name 'StarterTemplateWithWithRedisCacheTests' contains a duplicated 'With'. Consider renaming it to 'StarterTemplateWithRedisCacheTests' for clarity.

[RequiresDocker("Needs docker to start redis cache")]
[RequiresSSLCertificate]
public class StarterTemplateWithWithRedisCacheTests : StarterTemplateRunTestsBase<StarterTemplateWithRedisCacheFixture>

tests/Aspire.Workload.Tests/StarterTemplateWithWithRedisCacheTests_PreviousTFM.cs:9

[nitpick] The class name 'StarterTemplateWithWithRedisCacheTests_PreviousTFM' includes a duplicated 'With'. Consider renaming it to 'StarterTemplateWithRedisCacheTests_PreviousTFM' to eliminate redundancy.

[RequiresDocker("Needs docker to start redis cache")]
[RequiresSSLCertificate]
public class StarterTemplateWithWithRedisCacheTests_PreviousTFM : StarterTemplateRunTestsBase<StarterTemplateWithRedisCacheFixture_PreviousTFM>

tests/Aspire.Workload.Tests/StarterTemplateProjectNamesTests.cs

radical · 2025-03-11T04:58:06Z

@eerhardt The Final results job in the Templates workflow will need to be marked as Required too.

radical · 2025-03-11T05:09:40Z

Currently, we build the packages on each of the template test jobs to save time, instead of building once and uploading+downloading in the test jobs.

radical · 2025-03-11T05:43:25Z

We can explore using https://github.com/actions/cache in a follow-up.

tests/Aspire.Workload.Tests/StarterTemplateProjectNamesTests.cs

eerhardt · 2025-03-11T14:34:08Z

tests/Aspire.Workload.Tests/StarterTemplateProjectNamesTests.cs

+    }
+}
+
+// Individual class for each test framework so the tests can run in separate helix jobs


Why are these split, but BuildAndRunTemplateTests aren't?

These had a natural split based on the template argument for the test type. I can split the others in a follow up based on how long they take to run. But that timing will be affected once we start using the actions/cache.

.github/workflows/template-tests.yml

eerhardt · 2025-03-11T14:39:42Z

.github/workflows/template-tests.yml

+      testSessionTimeoutMs: 1200000
+      testHangTimeout: 12m


With a 20 minute session timeout, is having 12 minute timeout for each test too much?

Some tests do multiple builds, and runs of the project. I was using a 7m timeout earlier, but because of those tests I had to bump it.

eerhardt · 2025-03-11T14:42:26Z

.github/workflows/tests.yml

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


do the setup jobs take long enough to justify this duplication? how long are we talking about?

On some runs (yesterday for example) it was 40s vs 4m.

4 minutes to gather which tests to run??

I think it was nuget restore taking time essentially.

Hmm, I wonder if splitting this is actually going to help or hurt, since now it is 2 machines doing the restore instead of 1.

radical · 2025-03-11T19:34:51Z

The failing test is unrelated - #8013 . Re-running that.

eerhardt

Let's get this in so these tests are protected. We can make fix ups if necessary afterwards.

radical force-pushed the re-enable-workload-tests branch from 67059c6 to 779d04c Compare March 6, 2025 20:20

github-actions bot added the area-integrations Issues pertaining to Aspire Integrations packages label Mar 10, 2025

radical added 5 commits March 11, 2025 00:27

Extract enumerate-tests custom action from tests.yml

9b325e6

.. and use independent jobs to get list of tests for Windows, and Linux.

Add workload-tests.yml workflow with enumerate-template-tests action

3051248

Add RequiresSSLCertificate attribute

b9e0d12

.. and disable tests that need that, on windows/ci.

split tests to allow running in parallel

dc62e82

Cleanup workload tests

9e765f4

radical force-pushed the re-enable-workload-tests branch from ec89e29 to 9e765f4 Compare March 11, 2025 04:32

Make casing for environment variable names consistent

c4833fb

radical marked this pull request as ready for review March 11, 2025 04:49

Copilot bot review requested due to automatic review settings March 11, 2025 04:49

radical requested a review from eerhardt as a code owner March 11, 2025 04:49

radical requested a review from joperezr March 11, 2025 04:49

Copilot AI reviewed Mar 11, 2025

View reviewed changes

tests/Aspire.Workload.Tests/StarterTemplateProjectNamesTests.cs Outdated Show resolved Hide resolved

radical requested a review from davidfowl March 11, 2025 04:52

radical added 2 commits March 11, 2025 00:54

Fix a redundant With in the class name

df38d14

Fix a redundant With in the class name

d0ef258

radical mentioned this pull request Mar 6, 2025

Testing infrastructure tasks #7837

Open

8 tasks

radical added 6 commits March 11, 2025 03:20

Upload a combined test results artifacts

c94332f

Merge remote-tracking branch 'origin/main' into re-enable-workload-tests

75ec1ab

Differentiate windows and linux test results

7b86bbe

cleanup

fa49a39

Always upload the test results

911124b

Always upload the test results

007cc8d

eerhardt reviewed Mar 11, 2025

View reviewed changes

tests/Aspire.Workload.Tests/StarterTemplateProjectNamesTests.cs Outdated Show resolved Hide resolved

eerhardt reviewed Mar 11, 2025

View reviewed changes

.github/workflows/template-tests.yml Outdated Show resolved Hide resolved

eerhardt reviewed Mar 11, 2025

View reviewed changes

Address review feedback from @ eerhardt

df6c93e

radical mentioned this pull request Mar 11, 2025

Flaky test: WithHttpCommandTests.WithHttpCommand_EnablesCommandOnceResourceIsRunning #8013

Closed

radical enabled auto-merge (squash) March 11, 2025 19:35

radical mentioned this pull request Mar 11, 2025

add async suffix #8012

Closed

15 tasks

radical disabled auto-merge March 11, 2025 19:44

eerhardt approved these changes Mar 11, 2025

View reviewed changes

radical merged commit b092b22 into dotnet:main Mar 11, 2025
160 checks passed

radical deleted the re-enable-workload-tests branch March 11, 2025 20:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CI] Enable Workload tests for PR validation #7939

[CI] Enable Workload tests for PR validation #7939

radical commented Mar 6, 2025 •

edited

Loading

davidfowl commented Mar 7, 2025

radical commented Mar 11, 2025

Copilot AI left a comment

radical commented Mar 11, 2025

radical commented Mar 11, 2025

radical commented Mar 11, 2025 •

edited

Loading

eerhardt Mar 11, 2025

radical Mar 11, 2025

eerhardt Mar 11, 2025

radical Mar 11, 2025

eerhardt Mar 11, 2025

radical Mar 11, 2025

eerhardt Mar 11, 2025

radical Mar 11, 2025

eerhardt Mar 11, 2025

radical commented Mar 11, 2025

eerhardt left a comment

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

[CI] Enable Workload tests for PR validation #7939

[CI] Enable Workload tests for PR validation #7939

Conversation

radical commented Mar 6, 2025 • edited Loading

davidfowl commented Mar 7, 2025

radical commented Mar 11, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

PR Overview

Reviewed Changes

radical commented Mar 11, 2025

radical commented Mar 11, 2025

radical commented Mar 11, 2025 • edited Loading

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

radical commented Mar 11, 2025

eerhardt left a comment

Choose a reason for hiding this comment

radical commented Mar 6, 2025 •

edited

Loading

radical commented Mar 11, 2025 •

edited

Loading