Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ban function params for require.ErrorIs #1486

Merged
merged 5 commits into from
May 10, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading