Add new test suite for build #4464
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: TomSweeneyRedHat 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-ci-robot
added
approved
TomSweeneyRedHat
removed
the
do-not-merge/work-in-progress
TomSweeneyRedHat
changed the title
|
Finally happy green test buttons. This is GTG with a couple LGTM's |
|
LGTM |
test/e2e/build_test.go
Outdated
| podmanTest.Cleanup() | ||
| f := CurrentGinkgoTestDescription() | ||
| processTestResult(f) | ||
|
|
There was a problem hiding this comment.
since I can't seem to find much else...I might as well complain about this extraneous whitespace 😀
|
I see a bunch of other build-related directories under |
test/e2e/build_test.go
Outdated
|
|
||
| }) | ||
|
|
||
| It("podman build basic alpine", func() { |
There was a problem hiding this comment.
Mind changing the name and/or adding a quick comment for this test: It took me a moment to realize the rmi (below) is a secondary test...verifying a ancestor image is removable. It's a small thing, but might save someone else 2 minutes of head-scratching.
There was a problem hiding this comment.
That's interesting. I basically created this test as a "simple as it can get" to make sure it would work build and did the rmi soley to clean stuff up after. Will comment and/or rename.
There was a problem hiding this comment.
I tweaked the name and added a comment.
|
|
||
| session = podmanTest.PodmanNoCache([]string{"rmi", "-a", "-f"}) | ||
| session.WaitWithDefaultTimeout() | ||
| Expect(session.ExitCode()).To(Equal(0)) |
There was a problem hiding this comment.
There's a lot of duplication here, can it be simplified at all (for example) by feeding a list of string-lists, to a local function?
There was a problem hiding this comment.
I'm not sure how to shrink this down, but could be suffering with late evening brain. Maybe we can chat/point sometime over the next day or two on break?
There was a problem hiding this comment.
We didn't get a chance to chat in the F2F, If you've tips on condensing, happy to take them, but I'm not seeing it. If there are no other concerns, could we get this in and address in a follow up?
|
okay done looking. I appreciate the commit note about these only being basic tests, and explaining why. IMHO, the two near-by Nice work Tom, clear and easy to read. |
|
@cevich re: build directory. Short story, it's staying as if for now, long story, get yourself some popcorn. When we first developed the test suites for build, we weren't able to easily call and run buildah commands from within the test harness. So I created test/test_podman_build.sh and then created the test/build directory to hold all of the various Dockerfiles that it required. When we wanted to run a "build test" we did so 'by hand' outside of the CI. Then the good knight Sir Baude came to our little village and slew all of the test dragons that were plaguing our fair test harness. I recently came to the conclusion that we could now write e2e tests for the build process that would be part of the CI tests and then our village would be safe from all future dragons. So at this point, I think I'm going to eventually translate the tests from that build script into e2e tests in this new build_test.go file. Once that's complete I can either delete or move the old tests to a new location if you prefer, but for now I'd rather not touch that dragon in case it angrily awakens. |
Most build testing should be done in Buildah's test suites, but we should have a minimal amount of tests, especially testing the parts that are different like layers and squash. Also the CLI argument handling of things like the context directory that we've had issues reported. This first chunk does a basic test and then checks for context directory being a file and squash iterations. More to be added as time goes by. Signed-off-by: TomSweeneyRedHat <tsweeney@redhat.com>
|
Happy green tests buttons. @cevich and @edsantiago PTAL |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Most build testing should be done in Buildah's test
suites, but we should have a minimal amount of tests,
especially testing the parts that are different like
layers and squash. Also the CLI argument handling
of things like the context directory that we've had
issues reported.
This first chunk does a basic test and then checks for
context directory being a file and squash iterations.
More to be added as time goes by.
Signed-off-by: TomSweeneyRedHat tsweeney@redhat.com