More fixes for golangci-lint upgrade #94
Conversation
brandur
force-pushed
the
brandur-more-golang-ci-lint-fixes
branch
from
a23d11a
to
2efeeb7
Compare
| @@ -897,7 +897,7 @@ func Test_Client_InsertMany(t *testing.T) { | |||
|
|
|||
| jobs, err := bundle.queries.JobGetByKind(ctx, client.driver.GetDBPool(), (noOpArgs{}).Kind()) | |||
| require.NoError(t, err) | |||
| require.Len(t, jobs, 2, fmt.Sprintf("Expected to find exactly two jobs of kind: %s", (noOpArgs{}).Kind())) | |||
| require.Len(t, jobs, 2, "Expected to find exactly two jobs of kind: "+(noOpArgs{}).Kind()) //nolint:goconst | |||
There was a problem hiding this comment.
Added nolint on these lines ... I dunno about adding constants for assertion messages.
There was a problem hiding this comment.
Ah, I suspect you're running on brew install golangci-lint --HEAD whereas I'm running the latest bundled release again. I had to use HEAD at one point due to an OS upgrade iirc but that's resolved now.
Up to you if you want to move forward with this right now or wait til this gets officially released.
Hah, I swear I didn't. This is the exact command I ran today (does an upgrade take head into account?): Either way, these changes are very likely going to be coming in anyway, so probably not too harmful to get them a little early. |
So after #93 came in I ran my own `brew upgrade golangci-lint`, and I guess I got a new point release, because I got a whole bunch of new lint problems. I wish it was clearer which version I even got, but apparently golangci-lint is releasing HEAD versions to Homebrew: $ golangci-lint --version golangci-lint has version HEAD-185b2c7 built with go1.21.4 from 185b2c7 on 2023-11-29T17:26:42Z I fixed most of the problems, but I ended up disabling one of the features of `testifylint` that disallows uses of testify functions in goroutines, which would've required some very significant changes, and which in our project produces a false positive rate (of actual problems) of 100.0% as far as I can tell. See the description of this feature here: https://github.com/Antonboom/testifylint#go-require $ golangci-lint run --fix client_test.go:378:6: go-require: require must only be used in the goroutine running the test function (testifylint) require.NoError(t, err) ^ internal/baseservice/base_service_test.go:40:4: go-require: require must only be used in the goroutine running the test function (testifylint) require.FailNow(t, "Test case took too long to run (sleep statements should return immediately)") ^ internal/dbadapter/db_adapter_test.go:409:6: go-require: require must only be used in the goroutine running the test function (testifylint) require.NoError(t, err) ^ internal/riverinternaltest/riverinternaltest_test.go:78:8: go-require: TestTx contains assertions that must only be used in the goroutine running the test function (testifylint) _ = TestTx(ctx, t)
brandur
force-pushed
the
brandur-more-golang-ci-lint-fixes
branch
from
2efeeb7
to
8be1e1d
Compare
|
Ah, actually you're probably right. I likely installed |
|
Thx. I may have jumped the gun, but going to merge this since the work's done and these will be coming down the pipeline soon-ish anyway. |
So after #93 came in I ran my own
brew upgrade golangci-lint, and Iguess I got a new point release, because I got a whole bunch of new lint
problems. I wish it was clearer which version I even got, but apparently
golangci-lint is releasing HEAD versions to Homebrew:
I fixed most of the problems, but I ended up disabling one of the
features of
testifylintthat disallows uses of testify functions ingoroutines, which would've required some very significant changes, and
which in our project produces a false positive rate (of actual problems)
of 100.0% as far as I can tell. See the description of this feature
here:
https://github.com/Antonboom/testifylint#go-require