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

e2e tests: Containerfile presence #545

Closed
wants to merge 1 commit into from
Closed

e2e tests: Containerfile presence #545

wants to merge 1 commit into from

Conversation

slimreaper35
Copy link
Collaborator

Maintainers will complete the following section

  • Commit messages are descriptive enough
  • Code coverage from testing does not decrease and new code is covered
  • Docs updated (if applicable)
  • Docs links in the code are still valid (if docs were updated)

Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:

@slimreaper35
Copy link
Collaborator Author

slimreaper35 commented May 30, 2024

Are you okay with this change?
In the future, when creating new e2e tests, part of the repo can / should be Containerfile, too.

I think it will make life easier when testing locally. Also, I suppose users have their Containerfile in the same repo when using cachi2.

Signed-off-by: Michal Šoltis <[email protected]>
Comment on lines +405 to +410
containerfile = tmp_path.joinpath("Containerfile")
if not containerfile.exists():
container_folder = test_data_dir.joinpath(test_case, "container")
containerfile = container_folder.joinpath("Containerfile")

with build_image_for_test_case(tmp_path, str(containerfile), test_case) as test_image:
Copy link
Member

Choose a reason for hiding this comment

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

While I appreciate you're thinking of doing this in a compatible manner, it's actually going to lead to a very messy state where part of the tests will have the containerfile inside the e2e repo and the rest will have it in cachi2 integration test data. I'd prefer if this was proposed as a complex change == move the existing container files out of cachi2 and mandate that new e2e repos follow the proposed practice. Which brings me to #430 which this change would be nicely paired with that docs where this practice would be explained.

@slimreaper35 slimreaper35 deleted the container branch June 17, 2024 20:19
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.

2 participants