Dockerize go lint and tests#4765
Draft
johnsaigle wants to merge 6 commits into
Draft
Conversation
johnsaigle
force-pushed
the
dockerize-go-lint-and-tests
branch
from
46983c5 to
5c4d726
Compare
johnsaigle
requested review from
SEJeff,
bemic,
djb15,
evan-gray,
kcsongor,
mdulin2,
panoel and
pleasew8t
as code owners
Contributor
Author
|
Note: this PR should ideally be kept in sync with the golangci-lint version here: #4701 |
johnsaigle
force-pushed
the
dockerize-go-lint-and-tests
branch
from
758686d to
93bfc4f
Compare
pleasew8t
reviewed
May 5, 2026
|
|
||
| RUN ln -s /usr/local/lib/node_modules/cspell/bin.mjs /usr/local/bin/cspell | ||
|
|
||
| RUN useradd -u 1000 -U -m -d /home/lint lint && mkdir -p /tmp/go /tmp/go-build && chmod -R 1777 /tmp/go /tmp/go-build |
Contributor
There was a problem hiding this comment.
nit: Mind just adding a one-liner comment describing the command. Could probably split it up into three commands.
pleasew8t
reviewed
May 5, 2026
| ENV GOPATH=/tmp/go | ||
| ENV GOCACHE=/tmp/go-build | ||
|
|
||
| RUN mkdir -p /tmp/go /tmp/go-build && chmod -R 1777 /tmp/go /tmp/go-build |
pleasew8t
previously approved these changes
May 5, 2026
pleasew8t
left a comment
pleasew8t
left a comment
Contributor
There was a problem hiding this comment.
Ran both the lint and unit test Makefile commands, and it works fine. It would be nice to have the lint command clean itself up (I see the unit test docker command uses --rm, can lint do this as well?).
johnsaigle
force-pushed
the
dockerize-go-lint-and-tests
branch
from
15275a4 to
358edcc
Compare
djb15
reviewed
May 20, 2026
djb15
left a comment
djb15
left a comment
Collaborator
There was a problem hiding this comment.
Minor suggestions, looks ok otherwise
johnsaigle
force-pushed
the
dockerize-go-lint-and-tests
branch
2 times, most recently
from
868ede3 to
c6532d4
Compare
Wormhole is using go 1.24 as well as a slightly out of date version of golangci-lint. This can make it annoying to run unit tests, code coverage tools, or linting locally assuming that a dev wants to use more recent toolchains on their system. This commit - adds a new Dockerfile for running unit tests and test coverage tools - modifies the local Dockerfile for linting to be up-to-date - adds cspell to that container - adds `make` targets for each of these All of this should make it easier for devs to run tests/linting locally without needing to specify extra flags like `GOTOOLCHAIN=1.24.13` It also adds a few more safe entries to `.dockerignore`, though this should not affect the new Dockerfiles or the other ones in the repo, given that they don't use `COPY .` This commit also slightly bumps the sleep duration in the altpub tests as it is more likely to run too slowly to complete properly within a container. Ideally the test would be refactored to not use sleep, but that's not in-scope.
johnsaigle
force-pushed
the
dockerize-go-lint-and-tests
branch
from
c6532d4 to
6e1c644
Compare
Contributor
Author
|
Converting to draft mode as this has potential sequencing conflicts with other open PRs. See #4701 (comment) for more details. cc: @mdulin2 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wormhole is using go 1.24 as well as a slightly out of date version of
golangci-lint. This can make it annoying to run unit tests, code
coverage tools, or linting locally assuming that a dev wants to use more
recent toolchains on their system (e.g. by installing
goorgolangci-lintvia something like brew or apt).
This commit
maketargets for each of thesegoimportsto use the latest version that supports Go 1.24All of this should make it easier for devs to run tests/linting locally
without needing to specify extra flags like
GOTOOLCHAIN=1.24.13It also adds a few more safe entries to
.dockerignore, though thisshould not affect the new Dockerfiles or the other ones in the repo,
given that they don't use
COPY .This commit also slightly bumps the sleep duration in the altpub tests
as it is more likely to run too slowly to complete properly within a
container. Ideally the test would be refactored to not use sleep, but
that's not in-scope.
See also #4701 as this PR is now failing with lints related to that version.