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

Have better hygiene of e2e leftovers #1580

Merged
merged 1 commit into from May 17, 2024
Merged

Have better hygiene of e2e leftovers #1580

merged 1 commit into from May 17, 2024

Conversation

josvazg
Copy link
Collaborator

@josvazg josvazg commented May 10, 2024

TL;DR:

  • Make e2e clear rule also restore the helm chart repo.
  • Move some generated e2e tests to properly .gitgnored paths.

Context:
When running locally, e2e tests will leave some git tracked files modified or create new files on non git-ignored paths.

This is counter-ergonomic and even dangerous. I have been saved from uploading secrets to GitHub by the GitHub secret hook more than once, when I inadvertently included e2e generated files in my PR changes.

This change does not fix the root cause but is part of a series to incrementally fix the issue, which has several components.Because e2e tests are:

  • Generally not well behaved in where they generate files, they tend to leave them directly in the test dir at test/e2e or on some other directory that is not part of any path defined in .gitignore.

  • Make changes to the helm chart in place before helm chart tests, rather than copying the helm chart somewhere else, on a .gitignored path, and applying needed changes there instead, using the resulting copy rather than the original path.

  • Update the bundle.Dockerfile as well as directories bundle/, config/ and deploy. This is somehow related to the fact that many of these files are only updated at release time, rather than as soon as code changes actually imply them. In any case, e2e tests should not rely on making those changes in place, if at all. My hypothesis is we might want to remove scripts/int_local.sh & scripts/e2e_local.sh completely, make the CI call make e2e and make int-test directly and ensure each test is responsible for setting up all its required environment, rather than relying on a script to do so.

    • For instance, all e2e test asume CRDs are installed, but this is problematic for helm chart e2e tests, which should do the CRD install themselves. Right now we hack it by removing CRDs before the helm charts test, but it should be the other way around, install only as needed.

BUT, all this is better done in smaller incremental steps, so for now we add some small improvements.

All Submissions:

  • Have you signed our CLA?

Copy link
Contributor

github-actions bot commented May 10, 2024

Signed-off-by: jose.vazquez <jose.vazquez@mongodb.com>
@josvazg josvazg merged commit 228081e into main May 17, 2024
51 checks passed
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