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

test: add multi- and recursive-relationship E2Es, refactor test utils #2537

Merged
merged 5 commits into from
May 9, 2024

Conversation

palpatim
Copy link
Member

@palpatim palpatim commented May 8, 2024

Description of changes

This change adds E2Es to follow up on #2533

Additional changes

  • Refactor durations into a shared test utility, use throughout CDK tests; refactored some shared CDK test setups into their respective test implementation files
    • This refactor is responsible for the bulk of the file changes. You can view it separately in 86d0be3. Despite the size, I'm including it in this change so we can get the new duration handling into tests that we'll subsequently copy/paste.
  • Refactor configurable-stack to use a StackConfig structure for passing configs like prefix and an option to use the sandbox
  • Removed configurable-sandbox-stack since StackConfig now controls sandbox in configurable-stack
  • Add an optional postDeployWaitMs parameter to the CDK deploy script to allow tests to opt-in to a wait period to let resources finalize/propagate. This is a naive attempt to make the CDK-based SQL tests more stable by imitating the wait that we do during SQL database setup to let security group settings change. The justification is that many of the SQL failures I see locally are due to Unable to retrieve secret for database connection from Secrets Manager... errors. My hypothesis is that the secrets manager VPC service endpoint hasn't fully resolved by the time we start the test, because a subsequent test run in the same project works as expected.
    • Update: Hard to know if this helped or not. We still saw one persistent failure in references-sqlprimary-sqlrelated.test.ts that looks like the secrets manager timeout, so I'm not convinced this really fixes things. I'd like to leave it in for now and continue investigating.

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • E2E tests pass
    • assoc_field failed because of a timeout in the cdk destroy section, but all tests passed
    • references-sqlprimary-sqlrelated.test.ts failed because of the recurring failure when a project is first set up, but passes locally

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

@palpatim palpatim requested review from a team as code owners May 8, 2024 22:03
@palpatim palpatim marked this pull request as draft May 8, 2024 22:03
@palpatim palpatim force-pushed the palpatim.test.reference-validation-e2es branch from 262fade to 38a3e75 Compare May 9, 2024 01:08
@palpatim palpatim marked this pull request as ready for review May 9, 2024 01:15
Copy link
Member Author

@palpatim palpatim left a comment

Choose a reason for hiding this comment

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

marker review

@palpatim palpatim requested review from a team and removed request for a team May 9, 2024 16:52
@palpatim palpatim merged commit b07330c into main May 9, 2024
6 checks passed
@palpatim palpatim deleted the palpatim.test.reference-validation-e2es branch May 9, 2024 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants