Skip to content

Conversation

radical
Copy link
Member

@radical radical commented Aug 24, 2024

This ensures that for the internal pipeline the tests are run against the final packages produced, instead of building packages twice - once for testing, and then for publishing.

  • And track change in final packages being built before the workload tests are run

Fixes #5404

Microsoft Reviewers: Open in CodeFlow

@ghost ghost added the area-integrations Issues pertaining to Aspire Integrations packages label Aug 24, 2024
@radical radical added area-engineering-systems infrastructure helix infra engineering repo stuff and removed area-integrations Issues pertaining to Aspire Integrations packages labels Aug 24, 2024
@radical radical force-pushed the internal-build-pack branch from 23f3acf to 25294c2 Compare August 29, 2024 23:50
@radical radical marked this pull request as ready for review August 29, 2024 23:52
@radical radical requested a review from eerhardt as a code owner August 29, 2024 23:52
@radical radical requested a review from RussKie August 29, 2024 23:53
@radical
Copy link
Member Author

radical commented Aug 29, 2024

Copy link
Member

@joperezr joperezr left a comment

Choose a reason for hiding this comment

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

Changes look good to me, thanks for cleaning this up @radical! One thing that I still feel a bit nervous about is the step for installing the workload to the separate dotnet-install in the artifacts dir for testing. Main worry is that: A) we run that after the non-helix tests have passed, so potentially rebuilding something that was already tested, and B) we also build this after signing packages, which might potentially re-build things unintentionally and not sign them.

Copy link
Member

@joperezr joperezr left a comment

Choose a reason for hiding this comment

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

Meant to approve.

- script: ${{ parameters.buildScript }}
-restore -build
-pack
-sign $(_SignArgs)
Copy link
Member

Choose a reason for hiding this comment

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

NIT: Do we need to sign here? Can't we just keep the -sign param on the workloads build and have that sign everything?

Copy link
Member Author

Choose a reason for hiding this comment

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

TBD: follow up

@radical radical merged commit 6d4656d into dotnet:main Sep 3, 2024
11 checks passed
@radical radical deleted the internal-build-pack branch September 3, 2024 22:14
@github-actions github-actions bot locked and limited conversation to collaborators Oct 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-engineering-systems infrastructure helix infra engineering repo stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Internal Build should not rebuild packages after testing
2 participants