Skip to content

Conversation

Abogical
Copy link
Member

@Abogical Abogical commented Oct 10, 2025

Reason for this change

  • Previously, the script was executing the script under the cwd of the script package, when it should be at the head branch folder.
  • PATH was not passed on to integration test spawn command, leading to a ENOENT error as the integration test command could not find yarn.
  • Workflow trigger on label is misconfigured.

Description of changes

  • We'll use node explicitly to run the script without changing the cwd.
  • PATH env variable is now passed to integration test command.
  • Workflow trigger on label is fixed.

Describe any new or updated permissions being added

No new permissions were added.

Description of how you validated changes

Tested on my fork here: https://github.com/Abogical/aws-cdk/actions/runs/18465794651/job/52607352315?pr=15

Checklist


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@github-actions github-actions bot added the admired-contributor [Pilot] contributed between 13-24 PRs to the CDK label Oct 10, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team October 10, 2025 15:17
@github-actions github-actions bot added the p2 label Oct 10, 2025
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Oct 10, 2025
@Abogical Abogical force-pushed the integ-deployment-test-invoke-fix branch 5 times, most recently from 9eae906 to 933483c Compare October 13, 2025 09:10
- Previously, the script was executing the script under the cwd of the script package, when it should be at the head branch folder. We'll use node explicitly to run the script without changing the cwd.

- Fixes the label and event conditions that would trigger the workflow.

- Use STS to assume Atmosphere role as OIDC is not needed when we use the codebuild runner.
@Abogical Abogical force-pushed the integ-deployment-test-invoke-fix branch from 933483c to bdc479e Compare October 13, 2025 09:36
PATH environment variable is used to find yarn/node binaries. Without it, a ENOENT error is emitted when running yarn.
Running via node directly instead of lerna will keep the cwd at the root as expected.
@Abogical Abogical force-pushed the integ-deployment-test-invoke-fix branch from bdc479e to 5626551 Compare October 13, 2025 12:26
@Abogical Abogical changed the title ci: fix integration-test-deployment script invocation ci: fix integration-test-deployment scrip Oct 13, 2025
@Abogical Abogical changed the title ci: fix integration-test-deployment scrip ci: fix integration-test-deployment script Oct 13, 2025
@Abogical Abogical marked this pull request as ready for review October 13, 2025 13:03
@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Oct 13, 2025
fetch-depth: 0
path: head

- name: Configure AWS credentials
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have the OIDC? Unless there's some concern on it that i'm not aware of

Copy link
Member Author

Choose a reason for hiding this comment

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

OIDC added back in. I'm currently testing in my fork if it still works.

Copy link
Member Author

Choose a reason for hiding this comment

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

workflow_dispatch: {}
merge_group: {}
pull_request_target:
types:
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the default behaviour if we didn't mention any?

Copy link
Member Author

Choose a reason for hiding this comment

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

It won't listen to any labelling events by default. So if we add the pr/needs-integration-test-deployment label, the workflow will still be skipped.

"bump": "./bump.sh",
"build-all": "tsc -b"
"build-all": "tsc -b",
"atmosphere-integ-test": "lerna run build --scope @aws-cdk/integration-test-deployment && node tools/@aws-cdk/integration-test-deployment/bin/index.js"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it building? I believe the normal pattern here is another separate command building, then this one only running

Copy link
Member Author

@Abogical Abogical Oct 13, 2025

Choose a reason for hiding this comment

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

  • pkglint above in package.json follows the same pattern.
  • It'll be helpful as a developer to have one command to rebuild and test the script to allow for faster development.

@Abogical Abogical requested a review from gasolima October 13, 2025 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

admired-contributor [Pilot] contributed between 13-24 PRs to the CDK contribution/core This is a PR that came from AWS. p2 pr/needs-maintainer-review This PR needs a review from a Core Team Member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants