-
Notifications
You must be signed in to change notification settings - Fork 509
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Create buildx integration tests #1770
Conversation
c1da668
to
0092c3d
Compare
Can you move the first commit to another PR? |
ff251f9
to
0bf0ea0
Compare
c0a36a4
to
50735d1
Compare
@@ -61,6 +76,17 @@ FROM binaries-$TARGETOS AS binaries | |||
# enable scanning for this stage | |||
ARG BUILDKIT_SBOM_SCAN_STAGE=true | |||
|
|||
FROM gobase AS integration-test-base | |||
RUN apk add --no-cache docker runc containerd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed we should provide custom versions in our matrix when running integration tests. e.g., containerd 1.6/1.7 and moby 23/24. Pretty much like we do in BuildKit: https://github.com/moby/buildkit/blob/81d19ad945cdbf921d182c50fc91169b1375b44d/Dockerfile#L4-L6
Can be done in follow-up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@crazy-max any chance you could take a look at this one 🙏 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure happy to look at it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do this in a follow-up instead, there are more changes than I expected. Some in BuildKit moby/buildkit@master...crazy-max:buildkit:itg-dockerd to set extra envs (basically prepend path with right location like we do with containerd) and path to the alt binary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small naming comments. Generally looks good.
|
||
FROM moby/buildkit:$BUILDKIT_VERSION AS buildkit | ||
|
||
FROM gobase AS gotestsum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the purpose of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was pulled in from moby/buildkit#3727, but I wasn't actually using it. I've now enabled test failure annotations as in buildkit, see https://github.com/docker/buildx/actions/runs/5012653318.
ad968f2
to
950b483
Compare
2ff6b2e
to
a7d9630
Compare
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
🎉 |
Buildx currently doesn't have any integration tests - this has actively caused regressions and feature breakages in the past. This is only going to continue as the scope of buildx grows to include more functionality and support more scenarios.
To resolve this, this PR adds a collection of very basic integration tests. The idea is that we should test the command line interface, at the point of user access - avoiding testing surfaces that the user doesn't directly interact with. Currently, only a couple of very basic tests for each supported driver are run (
docker
,docker-container
,remote
) to test basic build functionality and showcase a working sandbox.Moving forward, we should work on adding matrix test combinations for experimental features, test high-level bake interactions, multi-platform, etc, etc.