chore(ci): fix breaking tests #8413
Conversation
github-actions
bot
added
the
area/integrations
joshua-goldstein
requested review from
akon-dey,
gcxml,
skrdgraph,
darkn3rd and
meghalims
as code owners
|
@skrdgraph This should fix the issue of failing workflows. |
joshua-goldstein
added
arm64
and removed
area/integrations
| @@ -1,4 +1,4 @@ | |||
| name: ci-dgraph-load-tests | |||
| name: ci-dgraph-load-tests-amd64 | |||
There was a problem hiding this comment.
@joshua-goldstein - can we remove this line change?
I had mentioned this to @adityasadalage, and indicated to specify the x64, and not change the name.
We will lose history when we make a name change. Also, the future plan is to use matrix and bring in all the runner types into this workflow.
There was a problem hiding this comment.
@skrdgraph yeah I wasn't sure on this point - I just reverted the change
There was a problem hiding this comment.
Yeah ci-dgraph-tests-arm64.yml pipeline was given the new name. Would that need to be reverted to accommodate the matrix change later on?
| run: | | ||
| #!/bin/bash | ||
| GOVERSION=$({ [ -f .go-version ] && cat .go-version; }) | ||
| echo "GOVERSION=$GOVERSION" >> $GITHUB_ENV |
There was a problem hiding this comment.
+1 ^ thanks for adding this, could you also do this for go-lint?
There was a problem hiding this comment.
@skrdgraph Sure, previously the linter yaml was skipping the Go installation step entirely with skip-go-installation: true . I just pushed changes that model the linter after the Badger linter setup (hence the version bumps). Let me know if that looks okay.
.github/workflows/ci-golang-lint.yml
Outdated
| - name: golang-lint | ||
| env: | ||
| # prevent OOM | ||
| GOGC: 10 | ||
| uses: golangci/golangci-lint-action@v2 | ||
| uses: golangci/golangci-lint-action@v3.2.0 |
There was a problem hiding this comment.
should we just do @latest here?
There was a problem hiding this comment.
Latest or perhaps @ v3 ? That way a major version upgrade wouldn't break the CI
There was a problem hiding this comment.
if it does break, we would know something changed in the action side. Times when this would break is when the underlying schema would change things the way this action is invoked or some new fields coming in.
Irrespective it's good to do latest here as well IMO
There was a problem hiding this comment.
Makes sense! I pinned it to @ latest
There was a problem hiding this comment.
Interesting, @ latest is not possible (see the failed run here). But @ v3 works and will pull the latest minor versions
There was a problem hiding this comment.
they do have a latest tag https://github.com/golangci/golangci-lint-action/releases <- here
There was a problem hiding this comment.
you can probably just do golangci/golangci-lint-action without a version, this is still a guess
.github/workflows/ci-golang-lint.yml
Outdated
| with: | ||
| # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. | ||
| version: v1.36 | ||
| version: v1.48 |
There was a problem hiding this comment.
also should we just do latest here?
There was a problem hiding this comment.
latest can work here - currently latest is v1.50.1 so not a big bump
There was a problem hiding this comment.
it's easier if we pin to latest. We shouldn't be doing latest pins on core code though.
The other thing is there may be some go version <> go-lang-ci version compatibility issues.
There was a problem hiding this comment.
Also pinned to latest. 👍
Problem
Since we added arm64 runners, they are being picked up by our workflows. Tests are failing after about 1 minute with exec format error (due to documented Badger/Ristretto issue).
For more info on runner labels see here.
Solution
We modify workflows to only pickup amd runners. We also pin the Go version.