Skip to content

Fix tests #9077

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix tests #9077

wants to merge 1 commit into from

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented May 2, 2025

Get the two failing tests running in GHA using the runsheet.

@RussKie RussKie requested a review from radical May 2, 2025 10:34
@github-actions github-actions bot added the area-engineering-systems infrastructure helix infra engineering repo stuff label May 2, 2025
@RussKie RussKie requested a review from captainsafia May 2, 2025 10:34
@RussKie
Copy link
Member Author

RussKie commented May 2, 2025

@captainsafia
Copy link
Member

@RussKie With this pattern, can we remove the change that I made in #7748 or is this change happening in a different run?

@RussKie
Copy link
Member Author

RussKie commented May 4, 2025

@RussKie With this pattern, can we remove the change that I made in #7748 or is this change happening in a different run?

Change you made (that inspired this change) happens on in a different workflow, which this workflow is going to replace.

@RussKie RussKie force-pushed the igveliko/fix_testrunner branch from 0831eaf to eba0481 Compare May 5, 2025 00:10
@RussKie RussKie self-assigned this May 5, 2025
@RussKie RussKie force-pushed the igveliko/fix_testrunner branch from eba0481 to 42232dd Compare May 5, 2025 01:54
@RussKie
Copy link
Member Author

RussKie commented May 5, 2025

This is ready to merge

@@ -38,6 +38,9 @@
<_CreateRunsheet Condition=" '$(_RequiresPackages)' != 'true' and '$(FullE2e)' != 'true' ">true</_CreateRunsheet>

<_CreateRunsheet Condition=" '$(MSBuildProjectName)' == 'Aspire.Templates.Tests' ">false</_CreateRunsheet>

<!-- FIXME: temporary workaround for https://github.com/Azure/azure-functions-dotnet-worker/issues/2969 -->
<_PreCommand Condition=" '$(MSBuildProjectName)' == 'Aspire.Playground.Tests' ">./dotnet.sh build &quot;%24(pwd)/playground/AzureFunctionsEndToEnd/AzureFunctionsEndToEnd.Functions/AzureFunctionsEndToEnd.Functions.csproj&quot; -c $(Configuration) /p:SkipUnstableEmulators=true &amp;&amp; </_PreCommand>
Copy link
Member

Choose a reason for hiding this comment

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

Now that we have these command lines being generated per project, we should have this kind of project-specific stuff in the test projects themselves.

Copy link
Member Author

Choose a reason for hiding this comment

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

This crossed my mind too, but I'm unsure how discoverable that may be.

Copy link
Member

Choose a reason for hiding this comment

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

This is not going to be a common thing to do. And documentation can help with discoverability. But the customizations belong with the specific projects, IMHO.

@radical
Copy link
Member

radical commented May 5, 2025

Could you please do a new run because of the changes pushed after the last one?

@RussKie RussKie force-pushed the igveliko/fix_testrunner branch from 42232dd to f81aeb3 Compare May 5, 2025 22:26
@RussKie
Copy link
Member Author

RussKie commented May 5, 2025

@radical
Copy link
Member

radical commented May 5, 2025

re:force-push, this makes it really hard to review changes on these PRs! Even looking at the Compare button on the commit because it includes all the changes including any merges with main.

Screenshot 2025-05-05 at 18 29 52

@RussKie
Copy link
Member Author

RussKie commented May 5, 2025

This is a clean rebase.
For diff use https://github.com/dotnet/aspire/pull/9077/files please.

@RussKie RussKie enabled auto-merge (squash) May 5, 2025 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-engineering-systems infrastructure helix infra engineering repo stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants