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

Add basic e2e test for RDS module #203

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add basic e2e test for RDS module #203

wants to merge 5 commits into from

Conversation

guineveresaenger
Copy link
Contributor

@guineveresaenger guineveresaenger commented Mar 11, 2025

Due to Create and Destroy times being > 5 minutes each, this test verifies Preview only.

Fixes #40

Copy link
Contributor

@corymhall corymhall left a comment

Choose a reason for hiding this comment

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

Due to Create and Destroy times being > 5 minutes each, this test verifies Preview only.

I think that length of time is fine for a test and I'm not sure testing without Create and Destroy is worth it. Diff is also an important piece to test which we lose without Create.

Copy link
Contributor

@corymhall corymhall left a comment

Choose a reason for hiding this comment

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

Based on the team discussion yesterday we will be creating separate example style tests #211

@t0yv0
Copy link
Member

t0yv0 commented Mar 14, 2025

That's true but would it be really easy to promote it to a full test that also does Up? We don't mind if it takes 5 min.

@guineveresaenger
Copy link
Contributor Author

@t0yv0 it's easy to promote, but it takes >5 minutes in either direction so we'd expect it to take 15 minutes.

@t0yv0
Copy link
Member

t0yv0 commented Mar 14, 2025

I'd say do it. We can tune parallelism up if we have a lot of these.

@guineveresaenger guineveresaenger force-pushed the rds branch 3 times, most recently from 16b51d7 to 0019699 Compare March 15, 2025 00:16
@guineveresaenger
Copy link
Contributor Author

Added the test to the other TypeScript tests. It's not significantly different from them.

corymhall added a commit that referenced this pull request Mar 17, 2025
This is a temprorary(?)/partial(?) fix for the inconsistent plan error.

As part of creating the examples for (#203) I was running into this
issue and noticed that all of the occurrences were due to the
`terraform_data` resources changing from `null` to `""`. Adding this
simple `try` function to always fallback to `""` seems to have fixed the
issue in a lot of cases.

This obviously doesn't fix the inconsistent plan issue in all cases, but
it gets us unstuck for creating example tests.

re #198
@guineveresaenger guineveresaenger changed the title Add preview "e2e" test for a basic RDS module Add basic e2e test for RDS module Mar 17, 2025
@guineveresaenger
Copy link
Contributor Author

The diff shown in the test happens on Preview and reproduces IRL. May be worth consolidating with a more real-world example.

@guineveresaenger
Copy link
Contributor Author

Waiting on changes from #179.

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.

Test terraform-aws-modules/rds/aws
3 participants