Skip to content

test: update workflow #1851

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.

Sign up for GitHub

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

Merged
crazy-max merged 9 commits into from
May 30, 2023
Merged

test: update workflow #1851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e762e46
ci: merge test workflow to build one
crazy-max May 29, 2023
73c17ef
ci: send test reports to codecov
crazy-max May 29, 2023
ecf8dd0
test: type integration not needed
crazy-max May 29, 2023
b9cf467
test: use trap to remove cache volume
crazy-max May 29, 2023
74b121b
test: opt to avoid building the integration tests image
crazy-max May 29, 2023
98dedd3
ci: cache integration tests base and build in dedicated step
crazy-max May 29, 2023
55377b2
make: test-unit and test-integration targets
crazy-max May 29, 2023
a4db138
docs: update CONTRIBUTING.md to run tests
crazy-max May 29, 2023
38b4eef
ci: make release and bin-image depend on test
crazy-max May 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions .github/CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,43 @@ commit automatically with `git commit -s`.

### Run the unit- and integration-tests

Running tests:

```bash
make test
```

This runs all unit and integration tests, in a containerized environment.
Locally, every package can be tested separately with standard Go tools, but
integration tests are skipped if local user doesn't have enough permissions or
worker binaries are not installed.

```bash
# run unit tests only
make test-unit

# run integration tests only
make test-integration

# test a specific package
TESTPKGS=./bake make test

# run all integration tests with a specific worker
TESTFLAGS="--run=//worker=docker-container -v" make test
```

> **Note**
>
> Set `TEST_KEEP_CACHE=1` for the test framework to keep external dependant
> images in a docker volume if you are repeatedly calling `make test`. This
> helps to avoid rate limiting on the remote registry side.

> **Note**
>
> If you are working behind a proxy, you can set some of or all
> `HTTP_PROXY=http://ip:port`, `HTTPS_PROXY=http://ip:port`, `NO_PROXY=http://ip:port`
> for the test framework to specify the proxy build args.

To enter a demo container environment and experiment, you may run:

```
Expand Down
94 changes: 85 additions & 9 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,63 @@ env:
BUILDKIT_IMAGE: "moby/buildkit:latest"
REPO_SLUG: "docker/buildx-bin"
DESTDIR: "./bin"
TEST_CACHE_SCOPE: "test"

jobs:
prepare-test:
runs-on: ubuntu-22.04
steps:
-
name: Checkout
uses: actions/checkout@v3
-
name: Set up QEMU
uses: docker/setup-qemu-action@v2
-
name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2
with:
version: ${{ env.BUILDX_VERSION }}
driver-opts: image=${{ env.BUILDKIT_IMAGE }}
buildkitd-flags: --debug
-
name: Build
uses: docker/bake-action@v3
with:
targets: integration-test-base
set: |
*.cache-from=type=gha,scope=${{ env.TEST_CACHE_SCOPE }}
*.cache-to=type=gha,scope=${{ env.TEST_CACHE_SCOPE }}

test:
runs-on: ubuntu-22.04
needs:
- prepare-test
env:
TESTFLAGS: "-v --parallel=6 --timeout=30m"
TESTFLAGS_DOCKER: "-v --parallel=1 --timeout=30m"
GOTESTSUM_FORMAT: "standard-verbose"
TEST_IMAGE_BUILD: "0"
TEST_IMAGE_ID: "buildx-tests"
strategy:
fail-fast: false
matrix:
worker:
- docker
- docker-container
- remote
pkg:
- ./tests
include:
- pkg: ./...
skip-integration-tests: 1
steps:
-
name: Checkout
uses: actions/checkout@v3
-
name: Set up QEMU
uses: docker/setup-qemu-action@v2
-
name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2
Expand All @@ -39,20 +88,44 @@ jobs:
driver-opts: image=${{ env.BUILDKIT_IMAGE }}
buildkitd-flags: --debug
-
name: Test
name: Build test image
uses: docker/bake-action@v3
with:
targets: test
targets: integration-test
set: |
*.cache-from=type=gha,scope=test
*.cache-to=type=gha,scope=test
*.cache-from=type=gha,scope=${{ env.TEST_CACHE_SCOPE }}
*.output=type=docker,name=${{ env.TEST_IMAGE_ID }}
-
name: Upload coverage
name: Test
run: |
export TEST_REPORT_SUFFIX=-${{ github.job }}-$(echo "${{ matrix.pkg }}-${{ matrix.skip-integration-tests }}-${{ matrix.worker }}" | tr -dc '[:alnum:]-\n\r' | tr '[:upper:]' '[:lower:]')
./hack/test
env:
TEST_DOCKERD: "${{ (matrix.worker == 'docker' || matrix.worker == 'docker-container') && '1' || '0' }}"
TESTFLAGS: "${{ (matrix.worker == 'docker' || matrix.worker == 'docker-container') && env.TESTFLAGS_DOCKER || env.TESTFLAGS }} --run=//worker=${{ matrix.worker }}$"
TESTPKGS: "${{ matrix.pkg }}"
SKIP_INTEGRATION_TESTS: "${{ matrix.skip-integration-tests }}"
-
name: Send to Codecov
if: always()
uses: codecov/codecov-action@v3
with:
directory: ${{ env.DESTDIR }}/coverage
directory: ./bin/testreports
-
name: Generate annotations
if: always()
uses: crazy-max/.github/.github/actions/gotest-annotations@1a64ea6d01db9a48aa61954cb20e265782c167d9
with:
directory: ./bin/testreports
-
name: Upload test reports
if: always()
uses: actions/upload-artifact@v3
with:
name: test-reports
path: ./bin/testreports

prepare:
prepare-binaries:
runs-on: ubuntu-22.04
outputs:
matrix: ${{ steps.platforms.outputs.matrix }}
Expand All @@ -73,11 +146,11 @@ jobs:
binaries:
runs-on: ubuntu-22.04
needs:
- prepare
- prepare-binaries
strategy:
fail-fast: false
matrix:
platform: ${{ fromJson(needs.prepare.outputs.matrix) }}
platform: ${{ fromJson(needs.prepare-binaries.outputs.matrix) }}
steps:
-
name: Prepare
Expand Down Expand Up @@ -115,6 +188,8 @@ jobs:

bin-image:
runs-on: ubuntu-22.04
needs:
- test
if: ${{ github.event_name != 'pull_request' && github.repository == 'docker/buildx' }}
steps:
-
Expand Down Expand Up @@ -166,6 +241,7 @@ jobs:
release:
runs-on: ubuntu-22.04
needs:
- test
- binaries
steps:
-
Expand Down
81 changes: 0 additions & 81 deletions .github/workflows/test.yml
View file
Open in desktop

This file was deleted.

10 changes: 9 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,15 @@ lint:

.PHONY: test
test:
$(BUILDX_CMD) bake test
./hack/test

.PHONY: test-unit
test-unit:
TESTPKGS=./... SKIP_INTEGRATION_TESTS=1 ./hack/test

.PHONY: test
test-integration:
TESTPKGS=./tests ./hack/test

.PHONY: validate-vendor
validate-vendor:
Expand Down
54 changes: 13 additions & 41 deletions hack/test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,60 +3,32 @@
set -eu -o pipefail

: "${BUILDX_CMD=docker buildx}"
: "${CACHE_FROM=}"
: "${CACHE_TO=}"

: ${TEST_INTEGRATION=}
: ${TEST_REPORT_SUFFIX=}
: ${TEST_KEEP_CACHE=}
: ${TEST_DOCKERD=}
: ${TEST_BUILDKIT_IMAGE=}

if [ -n "$CACHE_FROM" ]; then
for cfrom in $CACHE_FROM; do
setFlags+=(--set "*.cache-from=$cfrom")
done
fi
if [ -n "$CACHE_TO" ]; then
for cto in $CACHE_TO; do
setFlags+=(--set "*.cache-to=$cto")
done
fi

if [ "$#" == 0 ]; then TEST_INTEGRATION=1; fi
: "${TEST_IMAGE_BUILD=1}"
: "${TEST_IMAGE_ID=buildx-tests}"

while test $# -gt 0; do
case "$1" in
integration)
TEST_INTEGRATION=1
;;
*)
echo "unknown arg $1"
;;
esac
shift
done
Comment on lines -28 to -38
Copy link
Copy Markdown
Collaborator

@jedevc jedevc May 30, 2023

Choose a reason for hiding this comment

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

I think I'd like to keep this if possible, we still default to integration if nothing else is specified.

I think we're going to need to create another suite of integration tests at some point to test the complexities of driver creation, e.g. there are a lot of options for setting up the kubernetes driver. These tests don't test the commands package directly, so it would make more sense to split it out, so we can run them in parallel etc.

Copy link
Copy Markdown
Member Author

@crazy-max crazy-max May 30, 2023

Choose a reason for hiding this comment

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

I think I'd like to keep this if possible, we still default to integration if nothing else is specified.

On BuildKit we needed this special case because we got to test the frontend specifically and also the gateway and I don't think we need to have this kind of logic in buildx.

I think we're going to need to create another suite of integration tests at some point to test the complexities of driver creation, e.g. there are a lot of options for setting up the kubernetes driver. These tests don't test the commands package directly, so it would make more sense to split it out, so we can run them in parallel etc.

Right we just invoke buildx binary for the inspect command integration test atm so we don't cover the commands package but don't think we need other kind of invocation of the integrations tests container like we do in BuildKit.

Copy link
Copy Markdown
Collaborator

@jedevc jedevc May 30, 2023

Choose a reason for hiding this comment

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

On BuildKit we needed this special case because we got to test the frontend specifically and also the gateway and I don't think we need to have this kind of logic in buildx.

I think our special case will come from wanting to test the commands specifically, and then also test the driver creation specifically. We'll probably want different types of environment for both (e.g. the driver creation tests will probably need to configure buildkit servers with different combinations of tls options, or spin up unique k8s clusters per test).

We might not do that though, so I think it's ok to simplify this for now, and we can always add it back in later.

: "${TEST_REPORT_SUFFIX=}"
: "${TEST_KEEP_CACHE=}"
: "${TEST_DOCKERD=}"
: "${TEST_BUILDKIT_IMAGE=}"

iid="buildx-tests"
if [ "$TEST_IMAGE_BUILD" = "1" ]; then
${BUILDX_CMD} bake integration-test --set "*.output=type=docker,name=$TEST_IMAGE_ID"
fi

testReportsDir="$(pwd)/bin/testreports"
mkdir -p "$testReportsDir"
testReportsVol="-v $testReportsDir:/testreports"
gotestsumArgs="--format=standard-verbose --jsonfile=/testreports/go-test-report$TEST_REPORT_SUFFIX.json --junitfile=/testreports/junit-report$TEST_REPORT_SUFFIX.xml"
gotestArgs="-mod=vendor -coverprofile=/testreports/coverage-report$TEST_REPORT_SUFFIX.txt -covermode=atomic"

${BUILDX_CMD} bake integration-test "${setFlags[@]}" --set "*.output=type=docker,name=$iid"

cacheVolume="buildx-test-cache"
if ! docker container inspect "$cacheVolume" >/dev/null 2>/dev/null; then
docker create -v /root/.cache -v /root/.cache/registry -v /go/pkg/mod --name "$cacheVolume" alpine
fi

if [ "$TEST_INTEGRATION" == 1 ]; then
cid=$(docker create --rm -v /tmp $testReportsVol --volumes-from=$cacheVolume -e GITHUB_REF -e TEST_DOCKERD -e TEST_BUILDKIT_IMAGE -e SKIP_INTEGRATION_TESTS -e GOTESTSUM_FORMAT ${BUILDKIT_INTEGRATION_SNAPSHOTTER:+"-eBUILDKIT_INTEGRATION_SNAPSHOTTER"} -e BUILDKIT_REGISTRY_MIRROR_DIR=/root/.cache/registry --privileged $iid gotestsum $gotestsumArgs --packages="${TESTPKGS:-./...}" -- $gotestArgs ${TESTFLAGS:--v})
docker start -a -i $cid
fi

if [ "$TEST_KEEP_CACHE" != "1" ]; then
docker rm -v $cacheVolume
trap 'docker rm -v $cacheVolume' EXIT
Copy link
Copy Markdown
Collaborator

@jedevc jedevc May 30, 2023

Choose a reason for hiding this comment

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

FYI - this was pulled from buildkit's ./hack/test, so maybe we need an update there as well?

Copy link
Copy Markdown
Member Author

@crazy-max crazy-max May 30, 2023

Choose a reason for hiding this comment

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

Yes indeed I was looking at making some changes in BuildKit related to this 👍

fi

cid=$(docker create --rm -v /tmp $testReportsVol --volumes-from=$cacheVolume -e GITHUB_REF -e TEST_DOCKERD -e TEST_BUILDKIT_IMAGE -e SKIP_INTEGRATION_TESTS -e GOTESTSUM_FORMAT ${BUILDKIT_INTEGRATION_SNAPSHOTTER:+"-eBUILDKIT_INTEGRATION_SNAPSHOTTER"} -e BUILDKIT_REGISTRY_MIRROR_DIR=/root/.cache/registry --privileged $TEST_IMAGE_ID gotestsum $gotestsumArgs --packages="${TESTPKGS:-./...}" -- $gotestArgs ${TESTFLAGS:--v})
docker start -a -i $cid