Skip to content

Commit

Permalink
Ban usage of require.Equal when testing for length (ava-labs#1497)
Browse files Browse the repository at this point in the history
  • Loading branch information
dhrubabasu committed May 17, 2023
1 parent d146232 commit 0eb61fb
Show file tree
Hide file tree
Showing 15 changed files with 65 additions and 47 deletions.
2 changes: 1 addition & 1 deletion codec/test_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ func TestSliceWithEmptySerialization(codec GeneralCodec, t testing.TB) {
version, err := manager.Unmarshal(expected, &unmarshaled)
require.NoError(err)
require.Zero(version)
require.Equal(1000, len(unmarshaled.Arr))
require.Len(unmarshaled.Arr, 1000)
}

func TestSliceWithEmptySerializationOutOfMemory(codec GeneralCodec, t testing.TB) {
Expand Down
2 changes: 1 addition & 1 deletion database/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func TestNewSortsDatabases(t *testing.T) {
require.Zero(cmp, "incorrect version on previous database")

dbs := manager.GetDatabases()
require.Equal(len(vers), len(dbs))
require.Len(dbs, len(vers))

for i, db := range dbs {
cmp = db.Version.Compare(vers[i])
Expand Down
20 changes: 19 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 require_error_is_no_funcs_as_params single_import interface_compliance_nil require_equal_zero require_len_zero"}
TESTS=${TESTS:-"golangci_lint license_header require_error_is_no_funcs_as_params single_import interface_compliance_nil require_equal_zero require_len_zero require_equal_len"}

function test_golangci_lint {
go install -v github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.2
Expand Down Expand Up @@ -84,6 +84,24 @@ function test_require_len_zero {
fi
}

function test_require_equal_len {
# This should only flag if len(foo) is the *actual* val, not the expected val.
#
# These should *not* match:
# - require.Equal(len(foo), 2)
# - require.Equal(t, len(foo), 2)
#
# These should match:
# - require.Equal(2, len(foo))
# - require.Equal(t, 2, len(foo))
if grep -R -o -P --exclude-dir='scripts' 'require\.Equal\((t, )?.*, len\([^,]*$' .; then
echo ""
echo "Use require.Len instead of require.Equal when testing for length."
echo ""
return 1
fi
}

# Ref: https://go.dev/doc/effective_go#blank_implements
function test_interface_compliance_nil {
if grep -R -o -P '_ .+? = &.+?\{\}' .; then
Expand Down
2 changes: 1 addition & 1 deletion snow/engine/common/queue/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ func TestHandleJobWithMissingDependencyOnRunnableStack(t *testing.T) {
}

missingIDs := jobs.MissingIDs()
require.Equal(1, len(missingIDs))
require.Len(missingIDs, 1)

require.Equal(missingIDs[0], job0.ID())

Expand Down
40 changes: 20 additions & 20 deletions utils/buffer/unbounded_deque_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ func TestUnboundedSliceDequePushLeftPopLeft(t *testing.T) {
require.IsType(&unboundedSliceDeque[int]{}, bIntf)
b := bIntf.(*unboundedSliceDeque[int])
require.Zero(bIntf.Len())
require.Equal(2, len(b.data))
require.Len(b.data, 2)
require.Zero(b.left)
require.Equal(1, b.right)
require.Empty(b.List())
Expand All @@ -251,7 +251,7 @@ func TestUnboundedSliceDequePushLeftPopLeft(t *testing.T) {

b.PushLeft(1) // slice is [1,EMPTY]
require.Equal(1, b.Len())
require.Equal(2, len(b.data))
require.Len(b.data, 2)
require.Equal(1, b.left)
require.Equal(1, b.right)
require.Equal([]int{1}, b.List())
Expand All @@ -267,7 +267,7 @@ func TestUnboundedSliceDequePushLeftPopLeft(t *testing.T) {
// This causes a resize
b.PushLeft(2) // slice is [2,1,EMPTY,EMPTY]
require.Equal(2, b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(3, b.left)
require.Equal(2, b.right)
require.Equal([]int{2, 1}, b.List())
Expand All @@ -289,7 +289,7 @@ func TestUnboundedSliceDequePushLeftPopLeft(t *testing.T) {
// Tests left moving left with no wrap around.
b.PushLeft(3) // slice is [2,1,EMPTY,3]
require.Equal(3, b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(2, b.left)
require.Equal(2, b.right)
require.Equal([]int{3, 2, 1}, b.List())
Expand Down Expand Up @@ -318,7 +318,7 @@ func TestUnboundedSliceDequePushLeftPopLeft(t *testing.T) {
require.True(ok)
require.Equal(3, got)
require.Equal(2, b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(3, b.left)
require.Equal(2, b.right)
require.Equal([]int{2, 1}, b.List())
Expand All @@ -342,7 +342,7 @@ func TestUnboundedSliceDequePushLeftPopLeft(t *testing.T) {
require.True(ok)
require.Equal(2, got)
require.Equal(1, b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Zero(b.left)
require.Equal(2, b.right)
require.Equal([]int{1}, b.List())
Expand All @@ -361,7 +361,7 @@ func TestUnboundedSliceDequePushLeftPopLeft(t *testing.T) {
// Test left wrapping around to the right side.
b.PushLeft(2) // slice is [2,1,EMPTY,EMPTY]
require.Equal(2, b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(3, b.left)
require.Equal(2, b.right)
require.Equal([]int{2, 1}, b.List())
Expand All @@ -384,7 +384,7 @@ func TestUnboundedSliceDequePushLeftPopLeft(t *testing.T) {
require.True(ok)
require.Equal(2, got)
require.Equal(1, b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Zero(b.left)
require.Equal(2, b.right)
require.Equal([]int{1}, b.List())
Expand All @@ -396,7 +396,7 @@ func TestUnboundedSliceDequePushLeftPopLeft(t *testing.T) {
require.True(ok)
require.Equal(1, got)
require.Zero(b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(1, b.left)
require.Equal(2, b.right)
require.Empty(b.List())
Expand All @@ -419,7 +419,7 @@ func TestUnboundedSliceDequePushRightPopRight(t *testing.T) {
require.IsType(&unboundedSliceDeque[int]{}, bIntf)
b := bIntf.(*unboundedSliceDeque[int])
require.Zero(bIntf.Len())
require.Equal(2, len(b.data))
require.Len(b.data, 2)
require.Zero(b.left)
require.Equal(1, b.right)
require.Empty(b.List())
Expand All @@ -434,7 +434,7 @@ func TestUnboundedSliceDequePushRightPopRight(t *testing.T) {

b.PushRight(1) // slice is [1,EMPTY]
require.Equal(1, b.Len())
require.Equal(2, len(b.data))
require.Len(b.data, 2)
require.Zero(b.left)
require.Zero(b.right)
require.Equal([]int{1}, b.List())
Expand All @@ -453,7 +453,7 @@ func TestUnboundedSliceDequePushRightPopRight(t *testing.T) {
// This causes a resize
b.PushRight(2) // slice is [1,2,EMPTY,EMPTY]
require.Equal(2, b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(3, b.left)
require.Equal(2, b.right)
require.Equal([]int{1, 2}, b.List())
Expand All @@ -475,7 +475,7 @@ func TestUnboundedSliceDequePushRightPopRight(t *testing.T) {
// Tests right moving right with no wrap around
b.PushRight(3) // slice is [1,2,3,EMPTY]
require.Equal(3, b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(3, b.left)
require.Equal(3, b.right)
require.Equal([]int{1, 2, 3}, b.List())
Expand All @@ -502,7 +502,7 @@ func TestUnboundedSliceDequePushRightPopRight(t *testing.T) {
require.True(ok)
require.Equal(3, got)
require.Equal(2, b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(3, b.left)
require.Equal(2, b.right)
require.Equal([]int{1, 2}, b.List())
Expand All @@ -527,7 +527,7 @@ func TestUnboundedSliceDequePushRightPopRight(t *testing.T) {
require.True(ok)
require.Equal(2, got)
require.Equal(1, b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(3, b.left)
require.Equal(1, b.right)
require.Equal([]int{1}, b.List())
Expand All @@ -549,7 +549,7 @@ func TestUnboundedSliceDequePushRightPopRight(t *testing.T) {
require.True(ok)
require.Equal(1, got)
require.Zero(b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(3, b.left)
require.Zero(b.right)
require.Empty(b.List())
Expand All @@ -566,7 +566,7 @@ func TestUnboundedSliceDequePushRightPopRight(t *testing.T) {

b.PushLeft(1) // slice is [EMPTY,EMPTY,EMPTY,1]
require.Equal(1, b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(2, b.left)
require.Zero(b.right)
require.Equal([]int{1}, b.List())
Expand All @@ -587,7 +587,7 @@ func TestUnboundedSliceDequePushRightPopRight(t *testing.T) {
require.True(ok)
require.Equal(1, got)
require.Zero(b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(2, b.left)
require.Equal(3, b.right)
require.Empty(b.List())
Expand All @@ -604,7 +604,7 @@ func TestUnboundedSliceDequePushRightPopRight(t *testing.T) {
// Tests right wrapping around to the left
b.PushRight(2) // slice is [EMPTY,EMPTY,EMPTY,2]
require.Equal(1, b.Len())
require.Equal(4, len(b.data))
require.Len(b.data, 4)
require.Equal(2, b.left)
require.Zero(b.right)
require.Equal([]int{2}, b.List())
Expand Down Expand Up @@ -648,7 +648,7 @@ func FuzzUnboundedSliceDeque(f *testing.F) {
}

list := b.List()
require.Equal(len(input), len(list))
require.Len(list, len(input))
for i, n := range input {
require.Equal(n, list[i])
}
Expand Down
2 changes: 1 addition & 1 deletion utils/sampler/rand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,6 @@ func FuzzRNG(f *testing.F) {
mathRNG := rand.New(stdSource) //#nosec G404
stdVal := mathRNG.Int63n(int64(max + 1))
require.Equal(val, uint64(stdVal))
require.Equal(len(source.nums), len(stdSource.nums))
require.Len(stdSource.nums, len(source.nums))
})
}
1 change: 0 additions & 1 deletion utils/set/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ func TestSet(t *testing.T) {
s.Add(id1)
require.True(s.Contains(id1))
require.Len(s.List(), 1)
require.Equal(len(s.List()), 1)
require.Equal(id1, s.List()[0])

s.Clear()
Expand Down
16 changes: 8 additions & 8 deletions vms/platformvm/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ func TestGetCurrentValidators(t *testing.T) {

err := service.GetCurrentValidators(nil, &args, &response)
require.NoError(err)
require.Equal(len(genesis.Validators), len(response.Validators))
require.Len(response.Validators, len(genesis.Validators))

for _, vdr := range genesis.Validators {
found := false
Expand Down Expand Up @@ -650,7 +650,7 @@ func TestGetCurrentValidators(t *testing.T) {
args = GetCurrentValidatorsArgs{SubnetID: constants.PrimaryNetworkID}
err = service.GetCurrentValidators(nil, &args, &response)
require.NoError(err)
require.Equal(len(genesis.Validators), len(response.Validators))
require.Len(response.Validators, len(genesis.Validators))

// Make sure the delegator is there
found := false
Expand All @@ -676,7 +676,7 @@ func TestGetCurrentValidators(t *testing.T) {
require.Equal(vdr.NodeID, innerVdr.NodeID)

require.NotNil(innerVdr.Delegators)
require.Equal(1, len(*innerVdr.Delegators))
require.Len(*innerVdr.Delegators, 1)
delegator := (*innerVdr.Delegators)[0]
require.Equal(delegator.NodeID, innerVdr.NodeID)
require.Equal(uint64(delegator.StartTime), delegatorStartTime)
Expand All @@ -696,14 +696,14 @@ func TestGetCurrentValidators(t *testing.T) {
// Call getValidators
response = GetCurrentValidatorsReply{}
require.NoError(service.GetCurrentValidators(nil, &args, &response))
require.Equal(len(genesis.Validators), len(response.Validators))
require.Len(response.Validators, len(genesis.Validators))

for i := 0; i < len(response.Validators); i++ {
vdr := response.Validators[i].(pchainapi.PermissionlessValidator)
if vdr.NodeID != validatorNodeID {
for _, vdr := range response.Validators {
castVdr := vdr.(pchainapi.PermissionlessValidator)
if castVdr.NodeID != validatorNodeID {
continue
}
require.Equal(uint64(100000), uint64(*vdr.AccruedDelegateeReward))
require.Equal(uint64(100000), uint64(*castVdr.AccruedDelegateeReward))
}
}

Expand Down
3 changes: 2 additions & 1 deletion vms/platformvm/txs/executor/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ func TestNewImportTx(t *testing.T) {

unsignedTx := tx.Unsigned.(*txs.ImportTx)
require.NotEmpty(unsignedTx.ImportedInputs)
require.Equal(len(tx.Creds), len(unsignedTx.Ins)+len(unsignedTx.ImportedInputs), "should have the same number of credentials as inputs")
numInputs := len(unsignedTx.Ins) + len(unsignedTx.ImportedInputs)
require.Equal(len(tx.Creds), numInputs, "should have the same number of credentials as inputs")

totalIn := uint64(0)
for _, in := range unsignedTx.Ins {
Expand Down
4 changes: 2 additions & 2 deletions vms/platformvm/vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ func TestGenesis(t *testing.T) {
require.True(ok)

currentValidators := vdrSet.List()
require.Equal(len(currentValidators), len(genesisState.Validators))
require.Len(genesisState.Validators, len(currentValidators))

for _, key := range keys {
nodeID := ids.NodeID(key.PublicKey().Address())
Expand Down Expand Up @@ -2776,7 +2776,7 @@ func TestVM_GetValidatorSet(t *testing.T) {
if tt.expectedErr != nil {
return
}
require.Equal(len(tt.expectedVdrSet), len(gotVdrSet))
require.Len(gotVdrSet, len(tt.expectedVdrSet))
for nodeID, vdr := range tt.expectedVdrSet {
otherVdr, ok := gotVdrSet[nodeID]
require.True(ok)
Expand Down
2 changes: 1 addition & 1 deletion vms/platformvm/warp/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func TestGetCanonicalValidatorSet(t *testing.T) {
require.Equal(tt.expectedWeight, weight)

// These are pointers so have to test equality like this
require.Equal(len(tt.expectedVdrs), len(vdrs))
require.Len(vdrs, len(tt.expectedVdrs))
for i, expectedVdr := range tt.expectedVdrs {
gotVdr := vdrs[i]
expectedPKBytes := bls.PublicKeyToBytes(expectedVdr.PublicKey)
Expand Down
2 changes: 1 addition & 1 deletion x/merkledb/history_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ func Test_Change_List(t *testing.T) {

changes, err := db.history.getValueChanges(startRoot, endRoot, nil, nil, 8)
require.NoError(err)
require.Equal(8, len(changes.values))
require.Len(changes.values, 8)
}

func TestHistoryRecord(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion x/merkledb/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func Test_Node_Marshal(t *testing.T) {
require.NoError(t, err)
rootParsed, err := parseNode(newPath([]byte("")), data)
require.NoError(t, err)
require.Equal(t, 1, len(rootParsed.children))
require.Len(t, rootParsed.children, 1)

rootIndex := root.getSingleChildPath()[len(root.key)]
parsedIndex := rootParsed.getSingleChildPath()[len(rootParsed.key)]
Expand Down
2 changes: 1 addition & 1 deletion x/merkledb/proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func Test_Proof_Marshal_Errors(t *testing.T) {
}

func verifyPath(t *testing.T, path1, path2 []ProofNode) {
require.Equal(t, len(path1), len(path2))
require.Len(t, path2, len(path1))
for i := range path1 {
require.True(t, bytes.Equal(path1[i].KeyPath.Value, path2[i].KeyPath.Value))
require.Equal(t, path1[i].KeyPath.hasOddLength(), path2[i].KeyPath.hasOddLength())
Expand Down

0 comments on commit 0eb61fb

Please sign in to comment.