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

ci: validation and unit/integration tests #3100

Merged
merged 11 commits into from
Jan 3, 2023
Merged

Conversation

crazy-max
Copy link
Member

- What I did

Create GHA workflows to validate and run unit/integration tests. This should have the same behavior as CircleCI workflow with some enhancements to reduce build time:

  • run unit/integration tests in //
  • each validation (lint, fmt-proto, check protos, vendor) is also done in //

- How I did it

Dockerfile has been changed to handle proto, vendor, lint and fmt-proto validation so we don't need to build the whole image as it is currently. Also adds a bake definition to handle the new stages easily.

In follow-up we can remove the CircleCI workflow if we are happy with GHA ones.

- How to test it

- Description for the changelog

@crazy-max crazy-max force-pushed the ci-bake branch 4 times, most recently from d335145 to 5d94d74 Compare December 3, 2022 23:40
@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2022

Codecov Report

❗ No coverage uploaded for pull request base (master@67d2b1b). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #3100   +/-   ##
=========================================
  Coverage          ?   61.69%           
=========================================
  Files             ?      153           
  Lines             ?    31094           
  Branches          ?        0           
=========================================
  Hits              ?    19183           
  Misses            ?    10359           
  Partials          ?     1552           
Flag Coverage Δ
coverage 61.69% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@crazy-max crazy-max marked this pull request as ready for review December 3, 2022 23:49
Dockerfile Outdated Show resolved Hide resolved
@thaJeztah
Copy link
Member

This looks really good, thank you!!

One thing I'm trying to grasp is if this is "correct" or not (looking at the [no statements]);

docker run -t -v swarmkit-cache:/go -v /home/runner/work/swarmkit/swarmkit/:/go/src/github.com/docker/swarmkit moby/swarmkit make coverage-integration
🐳 coverage-integration
go test -race -tags "" -test.short -coverprofile="$(go list -f "{{.Dir}}" github.com/moby/swarmkit/v2/integration)/coverage.txt" -covermode=atomic github.com/moby/swarmkit/v2/integration
ok  	github.com/moby/swarmkit/v2/integration	63.878s	coverage: [no statements]
make[1]: Leaving directory '/home/runner/work/swarmkit/swarmkit'

After that, CodeCov seems to have found something (but perhaps it's empty?)

[2022-12-03T23:56:48.958Z] ['info'] => Project root located at: /home/runner/work/swarmkit/swarmkit
[2022-12-03T23:56:48.964Z] ['info'] -> No token specified or token is empty
[2022-12-03T23:56:48.977Z] ['info'] Searching for coverage files...
[2022-12-03T23:56:49.126Z] ['info'] Warning: Some files located via search were excluded from upload.
[2022-12-03T23:56:49.127Z] ['info'] If Codecov did not locate your files, please review https://docs.codecov.com/docs/supported-report-formats
[2022-12-03T23:56:49.128Z] ['info'] => Found 1 possible coverage files:
  integration/coverage.txt
[2022-12-03T23:56:49.128Z] ['info'] Processing /home/runner/work/swarmkit/swarmkit/integration/coverage.txt...

@crazy-max
Copy link
Member Author

crazy-max commented Dec 4, 2022

One thing I'm trying to grasp is if this is "correct" or not (looking at the [no statements]);

I was wondering that myself but looks to be the same with CircleCI: https://app.circleci.com/pipelines/github/moby/swarmkit/544/workflows/dfd51318-2439-4684-9750-4d67bf257f33/jobs/10927?invite=true#step-109-7

🐳 coverage-integration
go test -race -tags "" -test.short -coverprofile="$(go list -f "{{.Dir}}" github.com/moby/swarmkit/v2/integration)/coverage.txt" -covermode=atomic github.com/moby/swarmkit/v2/integration
ok  	github.com/moby/swarmkit/v2/integration	73.865s	coverage: [no statements]
CircleCI received exit code 0

Coverage looks empty too for integration pkg: https://app.circleci.com/pipelines/github/moby/swarmkit/538/workflows/6f6b9666-8fce-4de0-a768-c03210c5466c/jobs/10921?invite=true#step-110-99

    + ./integration/coverage.txt bytes=13

FROM --platform=$BUILDPLATFORM golang:${GO_VERSION}-bullseye AS gobase
ARG DEBIAN_FRONTEND
RUN apt-get update && apt-get install -y --no-install-recommends git make rsync
WORKDIR /go/src/github.com/docker/swarmkit
Copy link
Member

Choose a reason for hiding this comment

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

Should this be github.com/moby/swarmkit now? (module is named moby/swarmkit/v2)

Copy link
Member Author

@crazy-max crazy-max Dec 26, 2022

Choose a reason for hiding this comment

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

Would need to first update:

prefix = "github.com/docker/swarmkit/api"

otherwise:

#15 0.412 protoc -I.:/go/src/github.com/moby/swarmkit/v2/vendor/github.com/gogo/protobuf:/go/src/github.com/moby/swarmkit/v2/vendor:/go/src:/usr/local/include --gogoswarm_out=plugins=grpc+deepcopy+storeobject+raftproxy+authenticatedwrapper,import_path=github.com/moby/swarmkit/v2/api,Mgogoproto/gogo.proto=github.com/gogo/protobuf/gogoproto,Mgoogle/protobuf/any.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/descriptor.proto=github.com/gogo/protobuf/protoc-gen-gogo/descriptor,Mgoogle/protobuf/field_mask.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/duration.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/wrappers.proto=github.com/gogo/protobuf/types,Mgithub.com/docker/swarmkit/protobuf/plugin/plugin.proto=github.com/moby/swarmkit/v2/protobuf/plugin:/go/src /go/src/github.com/moby/swarmkit/v2/api/ca.proto /go/src/github.com/moby/swarmkit/v2/api/control.proto /go/src/github.com/moby/swarmkit/v2/api/dispatcher.proto /go/src/github.com/moby/swarmkit/v2/api/health.proto /go/src/github.com/moby/swarmkit/v2/api/logbroker.proto /go/src/github.com/moby/swarmkit/v2/api/objects.proto /go/src/github.com/moby/swarmkit/v2/api/raft.proto /go/src/github.com/moby/swarmkit/v2/api/resource.proto /go/src/github.com/moby/swarmkit/v2/api/snapshot.proto /go/src/github.com/moby/swarmkit/v2/api/specs.proto /go/src/github.com/moby/swarmkit/v2/api/types.proto /go/src/github.com/moby/swarmkit/v2/api/watch.proto
#15 0.415 github.com/docker/swarmkit/api/types.proto: File not found.
#15 0.415 github.com/docker/swarmkit/api/specs.proto: File not found.
#15 0.421 github.com/docker/swarmkit/protobuf/plugin/plugin.proto: File not found.

(can be done in follow-up)

Copy link
Member

Choose a reason for hiding this comment

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

oh! yes, proto; I recall now that I ran into that myself; perhaps something we should add a comment for to explain "why"

containerized.mk Outdated Show resolved Hide resolved
.github/workflows/validate.yml Show resolved Hide resolved
direct.mk Outdated Show resolved Hide resolved
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member

@dperny @corhere @neersighted PTAL

@dperny dperny merged commit 481f030 into moby:master Jan 3, 2023
@crazy-max crazy-max deleted the ci-bake branch January 3, 2023 18:14
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.

4 participants