[CI] Enable Workload tests for PR validation by radical · Pull Request #7939 · microsoft/aspire

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>

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.

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

+    }
+}
+
+// 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.

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

+      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

+  # 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 AI 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

Comment thread tests/Aspire.Workload.Tests/StarterTemplateProjectNamesTests.cs Outdated

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

Closed

25 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

Comment thread tests/Aspire.Workload.Tests/StarterTemplateProjectNamesTests.cs Outdated

eerhardt reviewed Mar 11, 2025

View reviewed changes

Comment thread .github/workflows/template-tests.yml Outdated

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 microsoft:main Mar 11, 2025

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

github-actions Bot locked and limited conversation to collaborators Apr 11, 2025

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

Uh oh!

Conversation

radical commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidfowl commented Mar 7, 2025

Uh oh!

radical commented Mar 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

PR Overview

Reviewed Changes

Uh oh!

Uh oh!

radical commented Mar 11, 2025

Uh oh!

radical commented Mar 11, 2025

Uh oh!

radical commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

radical commented Mar 11, 2025

Uh oh!

eerhardt left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

radical commented Mar 6, 2025 •

edited

Loading

radical commented Mar 11, 2025 •

edited

Loading