Skip to content
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

fix(integ-runner): cleanup tmp snapshot before running test #23773

Merged
merged 2 commits into from
Jan 20, 2023
Merged

fix(integ-runner): cleanup tmp snapshot before running test #23773

merged 2 commits into from
Jan 20, 2023

Conversation

corymhall
Copy link
Contributor

The integ-runner obtains information on how to run a test from the integ.json manifest file in the cloud assembly. In order to get this information for a new version of the test it first synthesizes the new cloud assembly only for the purpose of loading the integ manifest. It will then run the actual test which synthesizes again into the same cloud assembly directory.

We need to cleanup that first temporary cloud assembly before running the actual test to make sure we remove any leftover files. Here is one example of what might happen.

  1. Do the temporary synthesis and create the cloud assembly at cdk-integ.out.my-test.js/. This assembly has an asset with some hash asset.abcdefg.
  2. Do the synthesis while executing the test. The cloud assembly at cdk-integ.out.my-test.js is updated. This time the asset hash is different asset.123456. Now we have two assets in the cloud assembly and one asset.abcdefg is not being used!
  3. Test completes and copies the temporary snapshot to the final snapshot directory, including the old asset

I've also added back the lambda-nodejs/integ.dependencies-pnpm.ts integration test that was removed in #23728. With this fix the test will no longer check in the asset file. I also removed the Trigger from the test since that is what introduced the always changing diff.


All Submissions:

Adding new Construct Runtime Dependencies:

  • This PR adds new construct runtime dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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

The integ-runner obtains information on how to run a test from the
`integ.json` manifest file in the cloud assembly. In order to get this
information for a new version of the test it first synthesizes the new
cloud assembly _only for the purpose of loading the integ manifest_. It
will then run the actual test which synthesizes again into the same
cloud assembly directory.

We need to cleanup that first temporary cloud assembly _before_ running
the actual test to make sure we remove any leftover files. Here is one
example of what might happen.

1. Do the temporary synthesis and create the cloud assembly at
   `cdk-integ.out.my-test.js/`. This assembly has an asset with some
   hash `asset.abcdefg`.
2. Do the synthesis while executing the test. The cloud assembly at
   `cdk-integ.out.my-test.js` is updated. This time the asset hash is
   different `asset.123456`. Now we have two assets in the cloud
   assembly and one `asset.abcdefg` is not being used!
3. Test completes and copies the temporary snapshot to the final
   snapshot directory, including the old asset

I've also added back the `lambda-nodejs/integ.dependencies-pnpm.ts`
integration test that was removed in #23728. With this fix the test will
no longer check in the asset file. I also removed the Trigger from the
test since that is what introduced the always changing diff.
@gitpod-io
Copy link

gitpod-io bot commented Jan 20, 2023

@github-actions github-actions bot added the p2 label Jan 20, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team January 20, 2023 14:59
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jan 20, 2023
Copy link
Contributor

@mrgrain mrgrain left a comment

Choose a reason for hiding this comment

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

Looks good!

@mergify
Copy link
Contributor

mergify bot commented Jan 20, 2023

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 043ac52
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 366f2ab into aws:main Jan 20, 2023
@mergify
Copy link
Contributor

mergify bot commented Jan 20, 2023

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants