[CI] Enumerate test projects at build time for PR validation #7842
Conversation
tests/Shared/GetTestProjects.proj
Outdated
| @@ -0,0 +1,43 @@ | |||
| <Project DefaultTargets="GenerateTestProjectsJsonForGithubActions"> | |||
There was a problem hiding this comment.
I think I would much rather have this expressed in yaml and than in msbuild.
There was a problem hiding this comment.
Having it in msbuild allows us to control at the .csproj level whether to skip the tests, though that might not be a common scenario.
I was trying to do all this in yaml, but generating the json for the matrix was a pain. But I do have the code simplified now, so that json is simpler too, and we can try yaml.
There was a problem hiding this comment.
Hm keeping it in msbuild though means that we are sharing the same rules for helix, and for gh actions, which would keep that consistent.
There was a problem hiding this comment.
I hear you, but, copilot is kinda crap at writing msbuild and it's amazing at writing yaml. When I look at our csproj files wrapping shell commands its quite unmaintainable. This yaml file is long but very clean and really easy to understand. Maybe this is a fine middle ground but it feels really messy to me.
There was a problem hiding this comment.
By "shell commands" do you mean the json being emitted to the file? I should be able to get that out of this project, then it would just be evaluating all the msbuild projects to find the ones that should be built.
The alternative would be to pick up the test projects with globbing, and keeping an exclude list in the yaml.
There was a problem hiding this comment.
My bias is David's, an exclude list in the yaml sounds good. I like simple. (If we don't care about Helix)
There was a problem hiding this comment.
I don't have a strong opinion here, but this would have helped me avoid breaking the build last week when I added Aspire.Cli.
I know that the Azure SDK uses this to great effect to enable more elaborate integration testing scenarios.
One thing where this matrix generation capability would be useful is in running the same set of tests across both Docker AND Podman container runtimes pretty easily.
But I don't have a strong opinion.
There was a problem hiding this comment.
I think we should dynamically generate the list, I am just having a hard time having that logic be in msbuild 😄
There was a problem hiding this comment.
I'm going to stick with msbuild because:
- one way to control which tests get skipped by helix/gh-actions, OS, and docker/podman .
- All the build context is in msbuild, for deciding when to skip some tests.
For example, PR validation would catch a case where you should disable some test, but a change in the yaml won't cause the same test to get skipped on helix. And it would trigger a pipeline failure, then an issue being opened, and then the whole thing being fixed.
BUT if we really want it in yaml, then I can make that change too :)
There was a problem hiding this comment.
Agree msbuild > python (well only in .NET land). I wouldn't mind if we could move off helix altogether 😄
radical
added
the
NO-MERGE
|
Taking a hybrid route:
|
radical
force-pushed
the
enumerate-tests-prs
branch
2 times, most recently
from
2e81eb9 to
1414c25
Compare
- Move core steps to run the tests to a new reusable workflow `run-tests.yml` - Generate the list of test projects using the new `tests/Shared/GetTestProjects.proj` - Generate the test matrix json using `generate_test_matrix_json.py` and write it to `$GITHUB_OUTPUT` - Adjust `tests.yml` to essentially use `run-tests.yml` to run the test matrix generated above - Also, introduce a new property to control running tests on github actions `$(RunTestsOnGithubActions)`.
radical
force-pushed
the
enumerate-tests-prs
branch
from
1414c25 to
2519b38
Compare
No description provided.