feat: lint tests

.changelog/unreleased/chore/1906.md

@@ -0,0 +1 @@
+lint cometbft tests

.github/workflows/check-generated.yml

@@ -3,6 +3,7 @@
 # Note that we run these checks regardless whether the input files have
 # changed, because generated code can change in response to toolchain updates
 # even if no files in the repository are modified.
+
 name: Check generated code
 on:
  pull_request:
@@ -46,7 +47,7 @@ jobs:
 
  - uses: actions/checkout@v4
  with:
- fetch-depth: 1 # we need a .git directory to run git diff
+ fetch-depth: 1  # we need a .git directory to run git diff
 
  - name: "Check protobuf generated code"
  run: |

.golangci.yml

@@ -1,6 +1,6 @@
 run:
- tests: false
- timeout: 5m
+ tests: true
+ timeout: 10m
 
 linters:
  enable-all: true
@@ -20,13 +20,16 @@ linters:
  - forcetypeassert
  - funlen
  - gochecknoglobals
+ - goimports
+ - gocritic
  - gochecknoinits
  - gocognit
  - gocyclo
  - godox
  - goerr113
  - golint
  - gomnd
+ - gofmt
  - gomoddirectives
  - ifshort
  - interfacebloat
@@ -38,17 +41,21 @@ linters:
  - maligned
  - nestif
  - nilnil
+ - noctx
  - nlreturn
  - nonamedreturns
+ - testpackage
+ - paralleltest
+ - unparam
  - nosnakecase
  - predeclared
  - tagliatelle
  - typecheck
  - varnamelen
  - wrapcheck
+ - tparallel
  - wsl
  - scopelint
- - unparam
  - revive
 
 issues:
@@ -62,8 +69,8 @@ issues:
  - goimports
  - govet
  - stylecheck
- max-same-issues: 50
-
+ max-same-issues: 10000
+ max-issues-per-linter: 10000
 linters-settings:
  dogsled:
  max-blank-identifiers: 3

abci/client/socket_client_test.go

@@ -14,7 +14,6 @@ import (
  "github.com/cometbft/cometbft/abci/types"
  cmtrand "github.com/cometbft/cometbft/internal/rand"
  "github.com/cometbft/cometbft/internal/service"
- "github.com/stretchr/testify/assert"
  "github.com/stretchr/testify/require"
 )
 
@@ -38,7 +37,7 @@ func TestCalls(t *testing.T) {
  require.Fail(t, "No response arrived")
  case err, ok := <-resp:
  require.True(t, ok, "Must not close channel")
- assert.NoError(t, err, "This should return success")
+ require.NoError(t, err)
  }
 }
 
@@ -71,7 +70,7 @@ func TestHangingAsyncCalls(t *testing.T) {
  require.Fail(t, "No response arrived")
  case err, ok := <-resp:
  require.True(t, ok, "Must not close channel")
- assert.Error(t, err, "We should get EOF error")
+ require.Error(t, err, "We should get EOF error")
  }
 }
 
@@ -112,7 +111,7 @@ func TestBulk(t *testing.T) {
  // Send bulk request
  res, err := client.FinalizeBlock(context.Background(), rfb)
  require.NoError(t, err)
- require.Equal(t, numTxs, len(res.TxResults), "Number of txs doesn't match")
+ require.Len(t, res.TxResults, numTxs, "Number of txs doesn't match")
  for _, tx := range res.TxResults {
  require.Equal(t, uint32(0), tx.Code, "Tx failed")
  }

abci/example/kvstore/kvstore_test.go

@@ -9,7 +9,6 @@ import (
  abcicli "github.com/cometbft/cometbft/abci/client"
  abciserver "github.com/cometbft/cometbft/abci/server"
  "github.com/cometbft/cometbft/abci/types"
- "github.com/cometbft/cometbft/internal/service"
  "github.com/cometbft/cometbft/libs/log"
  "github.com/stretchr/testify/require"
 )
@@ -25,12 +24,16 @@ func TestKVStoreKV(t *testing.T) {
 
  kvstore := NewInMemoryApplication()
  tx := []byte(testKey + ":" + testValue)
- testKVStore(ctx, t, kvstore, tx, testKey, testValue)
+ testKVStore(ctx, t, kvstore, tx)
  tx = []byte(testKey + "=" + testValue)
- testKVStore(ctx, t, kvstore, tx, testKey, testValue)
+ testKVStore(ctx, t, kvstore, tx)
 }
 
-func testKVStore(ctx context.Context, t *testing.T, app types.Application, tx []byte, key, value string) {
+func testKVStore(ctx context.Context, t *testing.T, app types.Application, tx []byte) {
+ t.Helper()
+
+ value := "def"
+ key := "abc"
  checkTxResp, err := app.CheckTx(ctx, &types.CheckTxRequest{Tx: tx, Type: types.CHECK_TX_TYPE_CHECK})
  require.NoError(t, err)
  require.Equal(t, uint32(0), checkTxResp.Code)
@@ -41,7 +44,7 @@ func testKVStore(ctx context.Context, t *testing.T, app types.Application, tx []
  req := &types.FinalizeBlockRequest{Height: 1, Txs: ppResp.Txs}
  ar, err := app.FinalizeBlock(ctx, req)
  require.NoError(t, err)
- require.Equal(t, 1, len(ar.TxResults))
+ require.Len(t, ar.TxResults, 1)
  require.False(t, ar.TxResults[0].IsErr())
  // commit
  _, err = app.Commit(ctx, &types.CommitRequest{})
@@ -84,14 +87,14 @@ func TestPersistentKVStoreEmptyTX(t *testing.T) {
  reqCheck := types.CheckTxRequest{Tx: tx, Type: types.CHECK_TX_TYPE_CHECK}
  resCheck, err := kvstore.CheckTx(ctx, &reqCheck)
  require.NoError(t, err)
- require.Equal(t, resCheck.Code, CodeTypeInvalidTxFormat)
+ require.Equal(t, CodeTypeInvalidTxFormat, resCheck.Code)
 
  txs := make([][]byte, 0, 4)
  txs = append(txs, []byte("key=value"), []byte("key:val"), []byte(""), []byte("kee=value"))
  reqPrepare := types.PrepareProposalRequest{Txs: txs, MaxTxBytes: 10 * 1024}
  resPrepare, err := kvstore.PrepareProposal(ctx, &reqPrepare)
  require.NoError(t, err)
- require.Equal(t, len(reqPrepare.Txs)-1, len(resPrepare.Txs), "Empty transaction not properly removed")
+ require.Len(t, resPrepare.Txs, len(reqPrepare.Txs)-1, "Empty transaction not properly removed")
 }
 
 func TestPersistentKVStoreKV(t *testing.T) {
@@ -101,7 +104,7 @@ func TestPersistentKVStoreKV(t *testing.T) {
  kvstore := NewPersistentApplication(t.TempDir())
  key := testKey
  value := testValue
- testKVStore(ctx, t, kvstore, NewTx(key, value), key, value)
+ testKVStore(ctx, t, kvstore, NewTx(key, value))
 }
 
 func TestPersistentKVStoreInfo(t *testing.T) {
@@ -237,13 +240,13 @@ func TestClientServer(t *testing.T) {
  defer cancel()
  // set up socket app
  kvstore := NewInMemoryApplication()
- client, _, err := makeClientServer(t, kvstore, "kvstore-socket", "socket")
+ client, err := makeClientServer(t, kvstore, "kvstore-socket", "socket")
  require.NoError(t, err)
  runClientTests(ctx, t, client)
 
  // set up grpc app
  kvstore = NewInMemoryApplication()
- gclient, _, err := makeClientServer(t, kvstore, t.TempDir(), "grpc")
+ gclient, err := makeClientServer(t, kvstore, t.TempDir(), "grpc")
  require.NoError(t, err)
  runClientTests(ctx, t, gclient)
 }
@@ -256,6 +259,7 @@ func makeApplyBlock(
  diff []types.ValidatorUpdate,
  txs ...[]byte,
 ) {
+ t.Helper()
  // make and apply block
  height := int64(heightInt)
  hash := []byte("foo")
@@ -289,7 +293,8 @@ func valsEqual(t *testing.T, vals1, vals2 []types.ValidatorUpdate) {
  }
 }
 
-func makeClientServer(t *testing.T, app types.Application, name, transport string) (abcicli.Client, service.Service, error) {
+func makeClientServer(t *testing.T, app types.Application, name, transport string) (abcicli.Client, error) {
+ t.Helper()
  // Start the listener
  addr := fmt.Sprintf("unix://%s.sock", name)
  logger := log.TestingLogger()
@@ -298,7 +303,7 @@ func makeClientServer(t *testing.T, app types.Application, name, transport strin
  require.NoError(t, err)
  server.SetLogger(logger.With("module", "abci-server"))
  if err := server.Start(); err != nil {
- return nil, nil, err
+ return nil, err
  }
 
  t.Cleanup(func() {
@@ -312,7 +317,7 @@ func makeClientServer(t *testing.T, app types.Application, name, transport strin
  require.NoError(t, err)
  client.SetLogger(logger.With("module", "abci-client"))
  if err := client.Start(); err != nil {
- return nil, nil, err
+ return nil, err
  }
 
  t.Cleanup(func() {
@@ -321,15 +326,16 @@ func makeClientServer(t *testing.T, app types.Application, name, transport strin
  }
  })
 
- return client, server, nil
+ return client, nil
 }
 
 func runClientTests(ctx context.Context, t *testing.T, client abcicli.Client) {
+ t.Helper()
  // run some tests....
  tx := []byte(testKey + ":" + testValue)
- testKVStore(ctx, t, client, tx, testKey, testValue)
+ testKVStore(ctx, t, client, tx)
  tx = []byte(testKey + "=" + testValue)
- testKVStore(ctx, t, client, tx, testKey, testValue)
+ testKVStore(ctx, t, client, tx)
 }
 
 func TestTxGeneration(t *testing.T) {

abci/tests/client_server_test.go

@@ -6,7 +6,7 @@ import (
  abciclient "github.com/cometbft/cometbft/abci/client"
  "github.com/cometbft/cometbft/abci/example/kvstore"
  abciserver "github.com/cometbft/cometbft/abci/server"
- "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
 )
 
 func TestClientServerNoAddrPrefix(t *testing.T) {
@@ -17,19 +17,19 @@ func TestClientServerNoAddrPrefix(t *testing.T) {
  app := kvstore.NewInMemoryApplication()
 
  server, err := abciserver.NewServer(addr, transport, app)
- assert.NoError(t, err, "expected no error on NewServer")
+ require.NoError(t, err)
  err = server.Start()
- assert.NoError(t, err, "expected no error on server.Start")
+ require.NoError(t, err)
  t.Cleanup(func() {
  if err := server.Stop(); err != nil {
  t.Error(err)
  }
  })
 
  client, err := abciclient.NewClient(addr, transport, true)
- assert.NoError(t, err, "expected no error on NewClient")
+ require.NoError(t, err)
  err = client.Start()
- assert.NoError(t, err, "expected no error on client.Start")
+ require.NoError(t, err)
  t.Cleanup(func() {
  if err := client.Stop(); err != nil {
  t.Error(err)

abci/types/messages_test.go

@@ -9,11 +9,12 @@ import (
  cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1"
  "github.com/cosmos/gogoproto/proto"
  "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
 )
 
 func TestMarshalJSON(t *testing.T) {
  b, err := json.Marshal(&ExecTxResult{Code: 1})
- assert.NoError(t, err)
+ require.NoError(t, err)
  // include empty fields.
  assert.True(t, strings.Contains(string(b), "code"))
  r1 := CheckTxResponse{
@@ -30,11 +31,11 @@ func TestMarshalJSON(t *testing.T) {
  },
  }
  b, err = json.Marshal(&r1)
- assert.Nil(t, err)
+ require.NoError(t, err)
 
  var r2 CheckTxResponse
  err = json.Unmarshal(b, &r2)
- assert.Nil(t, err)
+ require.NoError(t, err)
  assert.Equal(t, r1, r2)
 }
 
@@ -48,11 +49,11 @@ func TestWriteReadMessageSimple(t *testing.T) {
  for _, c := range cases {
  buf := new(bytes.Buffer)
  err := WriteMessage(c, buf)
- assert.Nil(t, err)
+ require.NoError(t, err)
 
  msg := new(EchoRequest)
  err = ReadMessage(buf, msg)
- assert.Nil(t, err)
+ require.NoError(t, err)
 
  assert.True(t, proto.Equal(c, msg))
  }
@@ -70,11 +71,11 @@ func TestWriteReadMessage(t *testing.T) {
  for _, c := range cases {
  buf := new(bytes.Buffer)
  err := WriteMessage(c, buf)
- assert.Nil(t, err)
+ require.NoError(t, err)
 
  msg := new(cmtproto.Header)
  err = ReadMessage(buf, msg)
- assert.Nil(t, err)
+ require.NoError(t, err)
 
  assert.True(t, proto.Equal(c, msg))
  }
@@ -102,11 +103,11 @@ func TestWriteReadMessage2(t *testing.T) {
  for _, c := range cases {
  buf := new(bytes.Buffer)
  err := WriteMessage(c, buf)
- assert.Nil(t, err)
+ require.NoError(t, err)
 
  msg := new(CheckTxResponse)
  err = ReadMessage(buf, msg)
- assert.Nil(t, err)
+ require.NoError(t, err)
 
  assert.True(t, proto.Equal(c, msg))
  }

abci/types/types_test.go

@@ -40,7 +40,7 @@ func TestHashAndProveResults(t *testing.T) {
  require.NoError(t, err)
 
  valid := proofs[i].Verify(root, bz)
- assert.NoError(t, valid, "%d", i)
+ require.NoError(t, valid)
  }
 }
 

cmd/cometbft/commands/reindex_event_test.go

@@ -5,19 +5,17 @@ import (
  "errors"
  "testing"
 
- "github.com/spf13/cobra"
- "github.com/stretchr/testify/mock"
- "github.com/stretchr/testify/require"
-
  dbm "github.com/cometbft/cometbft-db"
-
  abcitypes "github.com/cometbft/cometbft/abci/types"
  cmtcfg "github.com/cometbft/cometbft/config"
  blockmocks "github.com/cometbft/cometbft/internal/state/indexer/mocks"
  "github.com/cometbft/cometbft/internal/state/mocks"
  txmocks "github.com/cometbft/cometbft/internal/state/txindex/mocks"
  "github.com/cometbft/cometbft/internal/test"
  "github.com/cometbft/cometbft/types"
+ "github.com/spf13/cobra"
+ "github.com/stretchr/testify/mock"
+ "github.com/stretchr/testify/require"
 )
 
 const (

cmd/cometbft/commands/reset_test.go

@@ -4,10 +4,9 @@ import (
  "path/filepath"
  "testing"
 
- "github.com/stretchr/testify/require"
-
  cfg "github.com/cometbft/cometbft/config"
  "github.com/cometbft/cometbft/privval"
+ "github.com/stretchr/testify/require"
 )
 
 func Test_ResetAll(t *testing.T) {