Skip to content

Commit

Permalink
ban function params for require.ErrorIs (ava-labs#1486)
Browse files Browse the repository at this point in the history
  • Loading branch information
dhrubabasu committed May 10, 2023
1 parent b870515 commit 7b8bbd6
Show file tree
Hide file tree
Showing 21 changed files with 158 additions and 78 deletions.
3 changes: 2 additions & 1 deletion codec/test_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ func TestRegisterStructTwice(codec GeneralCodec, t testing.TB) {
require := require.New(t)

require.NoError(codec.RegisterType(&MyInnerStruct{}))
require.ErrorIs(codec.RegisterType(&MyInnerStruct{}), ErrDuplicateType)
err := codec.RegisterType(&MyInnerStruct{})
require.ErrorIs(err, ErrDuplicateType)
}

func TestUInt32(codec GeneralCodec, t testing.TB) {
Expand Down
9 changes: 8 additions & 1 deletion scripts/lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ fi
# by default, "./scripts/lint.sh" runs all lint tests
# to run only "license_header" test
# TESTS='license_header' ./scripts/lint.sh
TESTS=${TESTS:-"golangci_lint license_header"}
TESTS=${TESTS:-"golangci_lint license_header require_error_is_no_funcs_as_params"}

function test_golangci_lint {
go install -v github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.2
Expand Down Expand Up @@ -51,6 +51,13 @@ function test_license_header {
"${files[@]}"
}

function test_require_error_is_no_funcs_as_params {
if grep -R -zo -P 'require.ErrorIs\(.+?\)[^\n]*\)\n' .; then
echo ""
return 1
fi
}

function run {
local test="${1}"
shift 1
Expand Down
3 changes: 2 additions & 1 deletion snow/choices/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ func TestStatusValid(t *testing.T) {
require.NoError(Rejected.Valid())
require.NoError(Processing.Valid())
require.NoError(Unknown.Valid())
require.ErrorIs(Status(math.MaxInt32).Valid(), errUnknownStatus)
err := Status(math.MaxInt32).Valid()
require.ErrorIs(err, errUnknownStatus)
}

func TestStatusDecided(t *testing.T) {
Expand Down
6 changes: 4 additions & 2 deletions snow/consensus/snowman/consensus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1751,7 +1751,8 @@ func ErrorOnAddDecidedBlock(t *testing.T, factory Factory) {
ParentV: Genesis.IDV,
HeightV: Genesis.HeightV + 1,
}
require.ErrorIs(sm.Add(context.Background(), block0), errDuplicateAdd)
err := sm.Add(context.Background(), block0)
require.ErrorIs(err, errDuplicateAdd)
}

func ErrorOnAddDuplicateBlockID(t *testing.T, factory Factory) {
Expand Down Expand Up @@ -1789,7 +1790,8 @@ func ErrorOnAddDuplicateBlockID(t *testing.T, factory Factory) {
}

require.NoError(sm.Add(context.Background(), block0))
require.ErrorIs(sm.Add(context.Background(), block1), errDuplicateAdd)
err := sm.Add(context.Background(), block1)
require.ErrorIs(err, errDuplicateAdd)
}

func gatherCounterGauge(t *testing.T, reg *prometheus.Registry) map[string]float64 {
Expand Down
3 changes: 2 additions & 1 deletion vms/platformvm/blocks/executor/standard_block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,8 @@ func TestBanffStandardBlockTimeVerification(t *testing.T) {
)
require.NoError(err)
block := env.blkManager.NewBlock(banffChildBlk)
require.ErrorIs(block.Verify(context.Background()), errBanffStandardBlockWithoutChanges)
err = block.Verify(context.Background())
require.ErrorIs(err, errBanffStandardBlockWithoutChanges)
}

{
Expand Down
9 changes: 6 additions & 3 deletions vms/platformvm/signer/proof_of_possession_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,20 @@ func TestProofOfPossession(t *testing.T) {
blsPOP, err = newProofOfPossession()
require.NoError(err)
blsPOP.ProofOfPossession = [bls.SignatureLen]byte{}
require.ErrorIs(blsPOP.Verify(), bls.ErrFailedSignatureDecompress)
err = blsPOP.Verify()
require.ErrorIs(err, bls.ErrFailedSignatureDecompress)

blsPOP, err = newProofOfPossession()
require.NoError(err)
blsPOP.PublicKey = [bls.PublicKeyLen]byte{}
require.ErrorIs(blsPOP.Verify(), bls.ErrFailedPublicKeyDecompress)
err = blsPOP.Verify()
require.ErrorIs(err, bls.ErrFailedPublicKeyDecompress)

newBLSPOP, err := newProofOfPossession()
require.NoError(err)
newBLSPOP.ProofOfPossession = blsPOP.ProofOfPossession
require.ErrorIs(newBLSPOP.Verify(), errInvalidProofOfPossession)
err = newBLSPOP.Verify()
require.ErrorIs(err, errInvalidProofOfPossession)
}

func TestNewProofOfPossessionDeterministic(t *testing.T) {
Expand Down
12 changes: 8 additions & 4 deletions vms/platformvm/txs/add_delegator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ func TestAddDelegatorTxSyntacticVerify(t *testing.T) {
)

// Case : signed tx is nil
require.ErrorIs(stx.SyntacticVerify(ctx), ErrNilSignedTx)
err = stx.SyntacticVerify(ctx)
require.ErrorIs(err, ErrNilSignedTx)

// Case : unsigned tx is nil
require.ErrorIs(addDelegatorTx.SyntacticVerify(ctx), ErrNilTx)
err = addDelegatorTx.SyntacticVerify(ctx)
require.ErrorIs(err, ErrNilTx)

validatorWeight := uint64(2022)
inputs := []*avax.TransferableInput{{
Expand Down Expand Up @@ -98,7 +100,8 @@ func TestAddDelegatorTxSyntacticVerify(t *testing.T) {

// Case: signed tx not initialized
stx = &Tx{Unsigned: addDelegatorTx}
require.ErrorIs(stx.SyntacticVerify(ctx), errSignedTxNotInitialized)
err = stx.SyntacticVerify(ctx)
require.ErrorIs(err, errSignedTxNotInitialized)

// Case: valid tx
stx, err = NewSigned(addDelegatorTx, Codec, signers)
Expand All @@ -119,7 +122,8 @@ func TestAddDelegatorTxSyntacticVerify(t *testing.T) {
addDelegatorTx.Wght = 2 * validatorWeight
stx, err = NewSigned(addDelegatorTx, Codec, signers)
require.NoError(err)
require.ErrorIs(stx.SyntacticVerify(ctx), errDelegatorWeightMismatch)
err = stx.SyntacticVerify(ctx)
require.ErrorIs(err, errDelegatorWeightMismatch)
addDelegatorTx.Wght = validatorWeight
}

Expand Down
6 changes: 4 additions & 2 deletions vms/platformvm/txs/add_subnet_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ func TestAddSubnetValidatorTxSyntacticVerify(t *testing.T) {
)

// Case : signed tx is nil
require.ErrorIs(stx.SyntacticVerify(ctx), ErrNilSignedTx)
err = stx.SyntacticVerify(ctx)
require.ErrorIs(err, ErrNilSignedTx)

// Case : unsigned tx is nil
require.ErrorIs(addSubnetValidatorTx.SyntacticVerify(ctx), ErrNilTx)
err = addSubnetValidatorTx.SyntacticVerify(ctx)
require.ErrorIs(err, ErrNilTx)

validatorWeight := uint64(2022)
subnetID := ids.ID{'s', 'u', 'b', 'n', 'e', 't', 'I', 'D'}
Expand Down
6 changes: 4 additions & 2 deletions vms/platformvm/txs/add_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@ func TestAddValidatorTxSyntacticVerify(t *testing.T) {
)

// Case : signed tx is nil
require.ErrorIs(stx.SyntacticVerify(ctx), ErrNilSignedTx)
err = stx.SyntacticVerify(ctx)
require.ErrorIs(err, ErrNilSignedTx)

// Case : unsigned tx is nil
require.ErrorIs(addValidatorTx.SyntacticVerify(ctx), ErrNilTx)
err = addValidatorTx.SyntacticVerify(ctx)
require.ErrorIs(err, ErrNilTx)

validatorWeight := uint64(2022)
rewardAddress := preFundedKeys[0].PublicKey().Address()
Expand Down
12 changes: 8 additions & 4 deletions vms/platformvm/txs/executor/reward_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ func TestRewardValidatorTxExecuteOnCommit(t *testing.T) {
Backend: &env.backend,
Tx: tx,
}
require.ErrorIs(tx.Unsigned.Visit(&txExecutor), ErrRemoveStakerTooEarly)
err = tx.Unsigned.Visit(&txExecutor)
require.ErrorIs(err, ErrRemoveStakerTooEarly)

// Advance chain timestamp to time that next validator leaves
env.state.SetTimestamp(stakerToRemove.EndTime)
Expand All @@ -79,7 +80,8 @@ func TestRewardValidatorTxExecuteOnCommit(t *testing.T) {
Backend: &env.backend,
Tx: tx,
}
require.ErrorIs(tx.Unsigned.Visit(&txExecutor), ErrRemoveWrongStaker)
err = tx.Unsigned.Visit(&txExecutor)
require.ErrorIs(err, ErrRemoveWrongStaker)

// Case 3: Happy path
tx, err = env.txBuilder.NewRewardValidatorTx(stakerToRemove.TxID)
Expand Down Expand Up @@ -159,7 +161,8 @@ func TestRewardValidatorTxExecuteOnAbort(t *testing.T) {
Backend: &env.backend,
Tx: tx,
}
require.ErrorIs(tx.Unsigned.Visit(&txExecutor), ErrRemoveStakerTooEarly)
err = tx.Unsigned.Visit(&txExecutor)
require.ErrorIs(err, ErrRemoveStakerTooEarly)

// Advance chain timestamp to time that next validator leaves
env.state.SetTimestamp(stakerToRemove.EndTime)
Expand All @@ -174,7 +177,8 @@ func TestRewardValidatorTxExecuteOnAbort(t *testing.T) {
Backend: &env.backend,
Tx: tx,
}
require.ErrorIs(tx.Unsigned.Visit(&txExecutor), ErrRemoveWrongStaker)
err = tx.Unsigned.Visit(&txExecutor)
require.ErrorIs(err, ErrRemoveWrongStaker)

// Case 3: Happy path
tx, err = env.txBuilder.NewRewardValidatorTx(stakerToRemove.TxID)
Expand Down
3 changes: 2 additions & 1 deletion vms/platformvm/txs/subnet_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ func TestSubnetValidatorVerifySubnetID(t *testing.T) {
Subnet: constants.PrimaryNetworkID,
}

require.ErrorIs(vdr.Verify(), errBadSubnetID)
err := vdr.Verify()
require.ErrorIs(err, errBadSubnetID)
}

// Happy path
Expand Down
5 changes: 3 additions & 2 deletions vms/proposervm/state_syncable_vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,9 +610,10 @@ func TestNoStateSummariesServedWhileRepairingHeightIndex(t *testing.T) {

// set height index to reindexing
proVM.hIndexer.MarkRepaired(false)
require.ErrorIs(proVM.VerifyHeightIndex(context.Background()), block.ErrIndexIncomplete)
err := proVM.VerifyHeightIndex(context.Background())
require.ErrorIs(err, block.ErrIndexIncomplete)

_, err := proVM.GetLastStateSummary(context.Background())
_, err = proVM.GetLastStateSummary(context.Background())
require.ErrorIs(err, block.ErrIndexIncomplete)

_, err = proVM.GetStateSummary(context.Background(), summaryHeight)
Expand Down
42 changes: 28 additions & 14 deletions vms/registry/vm_registerer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ func TestRegisterRegisterVMFails(t *testing.T) {
// We fail to register the VM
resources.mockManager.EXPECT().RegisterFactory(gomock.Any(), id, vmFactory).Times(1).Return(errTest)

require.ErrorIs(t, resources.registerer.Register(context.Background(), id, vmFactory), errTest)
err := resources.registerer.Register(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errTest)
}

// Tests Register if a VM doesn't actually implement VM.
Expand All @@ -48,7 +49,8 @@ func TestRegisterBadVM(t *testing.T) {
// Since this factory produces a bad vm, we should get an error.
vmFactory.EXPECT().New(logging.NoLog{}).Times(1).Return(vm, nil)

require.ErrorIs(t, resources.registerer.Register(context.Background(), id, vmFactory), errNotVM)
err := resources.registerer.Register(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errNotVM)
}

// Tests Register if creating endpoints for a VM fails + shutdown fails
Expand All @@ -65,7 +67,8 @@ func TestRegisterCreateHandlersAndShutdownFails(t *testing.T) {
vm.EXPECT().CreateStaticHandlers(gomock.Any()).Return(nil, errTest).Times(1)
vm.EXPECT().Shutdown(gomock.Any()).Return(errTest).Times(1)

require.ErrorIs(t, resources.registerer.Register(context.Background(), id, vmFactory), errTest)
err := resources.registerer.Register(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errTest)
}

// Tests Register if creating endpoints for a VM fails + shutdown succeeds
Expand All @@ -82,7 +85,8 @@ func TestRegisterCreateHandlersFails(t *testing.T) {
vm.EXPECT().CreateStaticHandlers(gomock.Any()).Return(nil, errTest).Times(1)
vm.EXPECT().Shutdown(gomock.Any()).Return(nil).Times(1)

require.ErrorIs(t, resources.registerer.Register(context.Background(), id, vmFactory), errTest)
err := resources.registerer.Register(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errTest)
}

// Tests Register if we fail to register the new endpoint on the server.
Expand Down Expand Up @@ -111,7 +115,8 @@ func TestRegisterAddRouteFails(t *testing.T) {
Times(1).
Return(errTest)

require.ErrorIs(t, resources.registerer.Register(context.Background(), id, vmFactory), errTest)
err := resources.registerer.Register(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errTest)
}

// Tests Register we can't find the alias for the newly registered vm
Expand Down Expand Up @@ -141,7 +146,8 @@ func TestRegisterAliasLookupFails(t *testing.T) {
Return(nil)
resources.mockManager.EXPECT().Aliases(id).Times(1).Return(nil, errTest)

require.ErrorIs(t, resources.registerer.Register(context.Background(), id, vmFactory), errTest)
err := resources.registerer.Register(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errTest)
}

// Tests Register if adding aliases for the newly registered vm fails
Expand Down Expand Up @@ -179,7 +185,8 @@ func TestRegisterAddAliasesFails(t *testing.T) {
).
Return(errTest)

require.ErrorIs(t, resources.registerer.Register(context.Background(), id, vmFactory), errTest)
err := resources.registerer.Register(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errTest)
}

// Tests Register if no errors are thrown
Expand Down Expand Up @@ -230,7 +237,8 @@ func TestRegisterWithReadLockRegisterVMFails(t *testing.T) {
// We fail to register the VM
resources.mockManager.EXPECT().RegisterFactory(gomock.Any(), id, vmFactory).Times(1).Return(errTest)

require.ErrorIs(t, resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory), errTest)
err := resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errTest)
}

// Tests RegisterWithReadLock if a VM doesn't actually implement VM.
Expand All @@ -245,7 +253,8 @@ func TestRegisterWithReadLockBadVM(t *testing.T) {
// Since this factory produces a bad vm, we should get an error.
vmFactory.EXPECT().New(logging.NoLog{}).Times(1).Return(vm, nil)

require.ErrorIs(t, resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory), errNotVM)
err := resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errNotVM)
}

// Tests RegisterWithReadLock if creating endpoints for a VM fails + shutdown fails
Expand All @@ -262,7 +271,8 @@ func TestRegisterWithReadLockCreateHandlersAndShutdownFails(t *testing.T) {
vm.EXPECT().CreateStaticHandlers(gomock.Any()).Return(nil, errTest).Times(1)
vm.EXPECT().Shutdown(gomock.Any()).Return(errTest).Times(1)

require.ErrorIs(t, resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory), errTest)
err := resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errTest)
}

// Tests RegisterWithReadLock if creating endpoints for a VM fails + shutdown succeeds
Expand All @@ -279,7 +289,8 @@ func TestRegisterWithReadLockCreateHandlersFails(t *testing.T) {
vm.EXPECT().CreateStaticHandlers(gomock.Any()).Return(nil, errTest).Times(1)
vm.EXPECT().Shutdown(gomock.Any()).Return(nil).Times(1)

require.ErrorIs(t, resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory), errTest)
err := resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errTest)
}

// Tests RegisterWithReadLock if we fail to register the new endpoint on the server.
Expand Down Expand Up @@ -308,7 +319,8 @@ func TestRegisterWithReadLockAddRouteWithReadLockFails(t *testing.T) {
Times(1).
Return(errTest)

require.ErrorIs(t, resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory), errTest)
err := resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errTest)
}

// Tests RegisterWithReadLock we can't find the alias for the newly registered vm
Expand Down Expand Up @@ -338,7 +350,8 @@ func TestRegisterWithReadLockAliasLookupFails(t *testing.T) {
Return(nil)
resources.mockManager.EXPECT().Aliases(id).Times(1).Return(nil, errTest)

require.ErrorIs(t, resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory), errTest)
err := resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errTest)
}

// Tests RegisterWithReadLock if adding aliases for the newly registered vm fails
Expand Down Expand Up @@ -376,7 +389,8 @@ func TestRegisterWithReadLockAddAliasesFails(t *testing.T) {
).
Return(errTest)

require.ErrorIs(t, resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory), errTest)
err := resources.registerer.RegisterWithReadLock(context.Background(), id, vmFactory)
require.ErrorIs(t, err, errTest)
}

// Tests RegisterWithReadLock if no errors are thrown
Expand Down
3 changes: 2 additions & 1 deletion vms/secp256k1fx/credential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ func TestCredentialVerify(t *testing.T) {
func TestCredentialVerifyNil(t *testing.T) {
require := require.New(t)
cred := (*Credential)(nil)
require.ErrorIs(cred.Verify(), ErrNilCredential)
err := cred.Verify()
require.ErrorIs(err, ErrNilCredential)
}

func TestCredentialSerialize(t *testing.T) {
Expand Down

0 comments on commit 7b8bbd6

Please sign in to comment.