Added github action to run a subset of tests #7328
Conversation
There was a problem hiding this comment.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/Aspire.Hosting.PostgreSQL/PostgresBuilderExtensions.cs:311
- [nitpick] The variable name 'mode' is ambiguous. It should be renamed to 'filePermissions'.
var mode = UnixFileMode.UserRead | UnixFileMode.UserWrite | UserExecute |
…etch basedirectory
| @@ -56,7 +56,7 @@ await WaitForAllTextAsync(app, | |||
| await app.StopAsync(); | |||
| } | |||
|
|
|||
| [Fact] | |||
| [Fact(Skip = "The project reference for this test is skipped in CI.")] | |||
There was a problem hiding this comment.
@captainsafia looks like this test hasn't been running on CI as the P2P has been conditioned out when -ci flag is passed to the build, is that intentional?
This change is just making it so that it is explicitly skipped so GHA doesn't try to run it but please let me know if that's wrong.
There was a problem hiding this comment.
@captainsafia looks like this test hasn't been running on CI as the P2P has been conditioned out when -ci flag is passed to the build, is that intentional?
💀 No...I'm a little surprised at this. Is that -ci flag actually set? Here are the test analytics for this test over the last 30 days off the main branch:
There was a problem hiding this comment.
That is for the whole workitem (meaning the whole test assembly as opposed to the single test) though, right?
There was a problem hiding this comment.
ci flag is set in our pipeline builds so that P2P wouldn't be included, meaning this test would fail like it did in this PR, so I'm guessing it is getting skipped by some different means.
There was a problem hiding this comment.
That is for the whole workitem (meaning the whole test assembly as opposed to the single test) though, right?
I accidentally cropped it from the image but its for the AzureFunctionsTest in particular:
I think we should remove the constraint in the project file since this test should be running.
There was a problem hiding this comment.
I did try that and it wasn't working for some reason. Anyway, for the sake of incremental progress I'll go ahead and not run playground tests in GHA for now and we can add that back in a follow up. That will enable the rest of the tests to start running in GHA.
There was a problem hiding this comment.
I'd be curious for what is happening in helix, I do have a hunch that the test is not running since I'm sure we pass -ci to our tests, so perhaps the Requires func attribute is making the test still shown as run but actually skipped dynamically? I can take a closer look
There was a problem hiding this comment.
Interesting, looks like playground tests actually don't build on the build machine and instead get built directly on Helix, and in there it seems like they don't pass -ci, so the test might indeed be running after all
|
I think this should be good to go now, and I'm planning on merging this later today. There are some pending items that I want to address in a follow-up:
FYI: @davidfowl |
Moving out CI tests to github actions (initial experiment #7073)