test/e2e: more fixes to not leak tmp files/dirs #22532
Conversation
Using /tmp means this file will be leaked and no deleted, switch to using the per test tempdir which is removed after the test. Signed-off-by: Paul Holzinger <[email protected]>
This should use the proper per test tempdir which works just as well for the purpose. Signed-off-by: Paul Holzinger <[email protected]>
Because the test left the image mounted the cleanup failed to remove the tmpdir as it contained an active mount point. Thus ensure we unmount the image again to prevent this leak. Signed-off-by: Paul Holzinger <[email protected]>
openshift-ci
bot
added
release-note-none
approved
|
Ephemeral COPR build failed. @containers/packit-build please check. |
| f, err := os.Create(privateFile) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| // Mark hello to be ignored in outerfile, but it should not be ignored. | ||
| _, err = f.WriteString("hello\n") | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| defer f.Close() | ||
|
|
||
| // Create .dockerignore which is a symlink to /tmp/private_file. | ||
| // Create .dockerignore which is a symlink to /tmp/.../private_file outside of the context dir. |
There was a problem hiding this comment.
Touching files in cwd is EVIL. Even if my package-e2e PR doesn't merge, I will be insisting on cleaning up this horrible practice:
https://github.com/containers/podman/pull/22395/files#diff-8b4d34c763f953caff8c6747e966aa1a7e869411747cd000370918887ba5b124
There was a problem hiding this comment.
yeah agreed, I didn't look that closely at the test just the leaked files.
FYI if you wonder where the oom file is created in the cwd when you run all tests see containers/conmon#504
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 9eefb5d
into
containers:main
stale-locking-app
bot
added
the
locked - please file new issue/PR
see commits
Does this PR introduce a user-facing change?