Skip to content

Commit

Permalink
Ban usage of require.Len when testing for length 0 (ava-labs#1496)
Browse files Browse the repository at this point in the history
  • Loading branch information
dhrubabasu authored May 17, 2023
1 parent 1bcab1f commit d146232
Show file tree
Hide file tree
Showing 19 changed files with 92 additions and 83 deletions.
20 changes: 10 additions & 10 deletions indexer/indexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,11 @@ func TestNewIndexer(t *testing.T) {
require.True(idxr.indexingEnabled)
require.True(idxr.allowIncompleteIndex)
require.NotNil(idxr.blockIndices)
require.Len(idxr.blockIndices, 0)
require.Empty(idxr.blockIndices)
require.NotNil(idxr.txIndices)
require.Len(idxr.txIndices, 0)
require.Empty(idxr.txIndices)
require.NotNil(idxr.vtxIndices)
require.Len(idxr.vtxIndices, 0)
require.Empty(idxr.vtxIndices)
require.NotNil(idxr.blockAcceptorGroup)
require.NotNil(idxr.txAcceptorGroup)
require.NotNil(idxr.vertexAcceptorGroup)
Expand Down Expand Up @@ -178,8 +178,8 @@ func TestIndexer(t *testing.T) {
require.Equal("index/chain1", server.bases[0])
require.Equal("/block", server.endpoints[0])
require.Len(idxr.blockIndices, 1)
require.Len(idxr.txIndices, 0)
require.Len(idxr.vtxIndices, 0)
require.Empty(idxr.txIndices)
require.Empty(idxr.vtxIndices)

// Accept a container
blkID, blkBytes := ids.GenerateTestID(), utils.RandomBytes(32)
Expand Down Expand Up @@ -236,9 +236,9 @@ func TestIndexer(t *testing.T) {
idxr = idxrIntf.(*indexer)
now = time.Now()
idxr.clock.Set(now)
require.Len(idxr.blockIndices, 0)
require.Len(idxr.txIndices, 0)
require.Len(idxr.vtxIndices, 0)
require.Empty(idxr.blockIndices)
require.Empty(idxr.txIndices)
require.Empty(idxr.vtxIndices)
require.True(idxr.hasRunBefore)
previouslyIndexed, err = idxr.previouslyIndexed(chain1Ctx.ChainID)
require.NoError(err)
Expand Down Expand Up @@ -445,7 +445,7 @@ func TestIncompleteIndex(t *testing.T) {
isIncomplete, err = idxr.isIncomplete(chain1Ctx.ChainID)
require.NoError(err)
require.True(isIncomplete)
require.Len(idxr.blockIndices, 0)
require.Empty(idxr.blockIndices)

// Close and re-open the indexer, this time with indexing enabled
require.NoError(config.DB.(*versiondb.Database).Commit())
Expand Down Expand Up @@ -523,5 +523,5 @@ func TestIgnoreNonDefaultChains(t *testing.T) {
// RegisterChain should return without adding an index for this chain
chainVM := mocks.NewMockChainVM(ctrl)
idxr.RegisterChain("chain1", chain1Ctx, chainVM)
require.Len(idxr.blockIndices, 0)
require.Empty(idxr.blockIndices)
}
4 changes: 2 additions & 2 deletions network/throttling/bandwidth_throttler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestBandwidthThrottler(t *testing.T) {
require.NotNil(throttler.limiters)
require.Equal(config.RefillRate, throttler.RefillRate)
require.Equal(config.MaxBurstSize, throttler.MaxBurstSize)
require.Len(throttler.limiters, 0)
require.Empty(throttler.limiters)

// Add a node
nodeID1 := ids.GenerateTestNodeID()
Expand All @@ -40,7 +40,7 @@ func TestBandwidthThrottler(t *testing.T) {

// Remove the node
throttler.RemoveNode(nodeID1)
require.Len(throttler.limiters, 0)
require.Empty(throttler.limiters)

// Add the node back
throttler.AddNode(nodeID1)
Expand Down
2 changes: 1 addition & 1 deletion network/throttling/inbound_msg_buffer_throttler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestMsgBufferThrottler(t *testing.T) {
throttler.release(nodeID1)
throttler.release(nodeID1)
throttler.release(nodeID1)
require.Len(throttler.nodeToNumProcessingMsgs, 0)
require.Empty(throttler.nodeToNumProcessingMsgs)
}

// Test inboundMsgBufferThrottler when an acquire is cancelled
Expand Down
20 changes: 10 additions & 10 deletions network/throttling/inbound_msg_byte_throttler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,16 +139,16 @@ func TestInboundMsgByteThrottler(t *testing.T) {
throttler.Acquire(context.Background(), 1, vdr1ID)
require.Equal(config.AtLargeAllocSize-1, throttler.remainingAtLargeBytes)
require.Equal(config.VdrAllocSize, throttler.remainingVdrBytes)
require.Len(throttler.nodeToVdrBytesUsed, 0)
require.Empty(throttler.nodeToVdrBytesUsed)
require.Len(throttler.nodeToAtLargeBytesUsed, 1)
require.Equal(uint64(1), throttler.nodeToAtLargeBytesUsed[vdr1ID])

// Release the bytes
throttler.release(&msgMetadata{msgSize: 1}, vdr1ID)
require.Equal(config.AtLargeAllocSize, throttler.remainingAtLargeBytes)
require.Equal(config.VdrAllocSize, throttler.remainingVdrBytes)
require.Len(throttler.nodeToVdrBytesUsed, 0)
require.Len(throttler.nodeToAtLargeBytesUsed, 0)
require.Empty(throttler.nodeToVdrBytesUsed)
require.Empty(throttler.nodeToAtLargeBytesUsed)

// Use all the at-large allocation bytes and 1 of the validator allocation bytes
// Should return immediately.
Expand All @@ -170,7 +170,7 @@ func TestInboundMsgByteThrottler(t *testing.T) {
require.Equal(config.VdrAllocSize/2, throttler.nodeToVdrBytesUsed[vdr2ID])
require.Len(throttler.nodeToVdrBytesUsed, 2)
require.Len(throttler.nodeToAtLargeBytesUsed, 1)
require.Len(throttler.nodeToWaitingMsgID, 0)
require.Empty(throttler.nodeToWaitingMsgID)
require.Zero(throttler.waitingToAcquire.Len())

// vdr1 should be able to acquire the rest of the validator allocation
Expand Down Expand Up @@ -256,14 +256,14 @@ func TestInboundMsgByteThrottler(t *testing.T) {
require.Len(throttler.nodeToVdrBytesUsed, 1)
require.Zero(throttler.nodeToVdrBytesUsed[vdr1ID])
require.Equal(config.AtLargeAllocSize/2-2, throttler.remainingAtLargeBytes)
require.Len(throttler.nodeToWaitingMsgID, 0)
require.Empty(throttler.nodeToWaitingMsgID)
require.Zero(throttler.waitingToAcquire.Len())

// Non-validator should be able to take the rest of the at-large bytes
throttler.Acquire(context.Background(), config.AtLargeAllocSize/2-2, nonVdrID)
require.Zero(throttler.remainingAtLargeBytes)
require.Equal(config.AtLargeAllocSize/2-1, throttler.nodeToAtLargeBytesUsed[nonVdrID])
require.Len(throttler.nodeToWaitingMsgID, 0)
require.Empty(throttler.nodeToWaitingMsgID)
require.Zero(throttler.waitingToAcquire.Len())

// But should block on subsequent Acquires
Expand Down Expand Up @@ -292,15 +292,15 @@ func TestInboundMsgByteThrottler(t *testing.T) {

require.Zero(throttler.nodeToAtLargeBytesUsed[vdr2ID])
require.Equal(config.VdrAllocSize, throttler.remainingVdrBytes)
require.Len(throttler.nodeToVdrBytesUsed, 0)
require.Empty(throttler.nodeToVdrBytesUsed)
require.Zero(throttler.remainingAtLargeBytes)
require.NotContains(throttler.nodeToWaitingMsgID, nonVdrID)
require.Zero(throttler.waitingToAcquire.Len())

// Release all of vdr1's messages
throttler.release(&msgMetadata{msgSize: 1}, vdr1ID)
throttler.release(&msgMetadata{msgSize: config.AtLargeAllocSize/2 - 1}, vdr1ID)
require.Len(throttler.nodeToVdrBytesUsed, 0)
require.Empty(throttler.nodeToVdrBytesUsed)
require.Equal(config.VdrAllocSize, throttler.remainingVdrBytes)
require.Equal(config.AtLargeAllocSize/2, throttler.remainingAtLargeBytes)
require.Zero(throttler.nodeToAtLargeBytesUsed[vdr1ID])
Expand All @@ -311,10 +311,10 @@ func TestInboundMsgByteThrottler(t *testing.T) {
throttler.release(&msgMetadata{msgSize: 1}, nonVdrID)
throttler.release(&msgMetadata{msgSize: 1}, nonVdrID)
throttler.release(&msgMetadata{msgSize: config.AtLargeAllocSize/2 - 2}, nonVdrID)
require.Len(throttler.nodeToVdrBytesUsed, 0)
require.Empty(throttler.nodeToVdrBytesUsed)
require.Equal(config.VdrAllocSize, throttler.remainingVdrBytes)
require.Equal(config.AtLargeAllocSize, throttler.remainingAtLargeBytes)
require.Len(throttler.nodeToAtLargeBytesUsed, 0)
require.Empty(throttler.nodeToAtLargeBytesUsed)
require.Zero(throttler.nodeToAtLargeBytesUsed[nonVdrID])
require.NotContains(throttler.nodeToWaitingMsgID, nonVdrID)
require.Zero(throttler.waitingToAcquire.Len())
Expand Down
14 changes: 7 additions & 7 deletions network/throttling/outbound_msg_throttler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,16 @@ func TestSybilOutboundMsgThrottler(t *testing.T) {
require.True(acquired)
require.Equal(config.AtLargeAllocSize-1, throttler.remainingAtLargeBytes)
require.Equal(config.VdrAllocSize, throttler.remainingVdrBytes)
require.Len(throttler.nodeToVdrBytesUsed, 0)
require.Empty(throttler.nodeToVdrBytesUsed)
require.Len(throttler.nodeToAtLargeBytesUsed, 1)
require.Equal(uint64(1), throttler.nodeToAtLargeBytesUsed[vdr1ID])

// Release the bytes
throttlerIntf.Release(msg, vdr1ID)
require.Equal(config.AtLargeAllocSize, throttler.remainingAtLargeBytes)
require.Equal(config.VdrAllocSize, throttler.remainingVdrBytes)
require.Len(throttler.nodeToVdrBytesUsed, 0)
require.Len(throttler.nodeToAtLargeBytesUsed, 0)
require.Empty(throttler.nodeToVdrBytesUsed)
require.Empty(throttler.nodeToAtLargeBytesUsed)

// Use all the at-large allocation bytes and 1 of the validator allocation bytes
msg = testMsgWithSize(ctrl, config.AtLargeAllocSize+1)
Expand Down Expand Up @@ -142,24 +142,24 @@ func TestSybilOutboundMsgThrottler(t *testing.T) {
throttlerIntf.Release(msg, vdr2ID)
require.Zero(throttler.nodeToAtLargeBytesUsed[vdr2ID])
require.Equal(config.VdrAllocSize, throttler.remainingVdrBytes)
require.Len(throttler.nodeToVdrBytesUsed, 0)
require.Empty(throttler.nodeToVdrBytesUsed)
require.Zero(throttler.remainingAtLargeBytes)

// Release all of vdr1's messages
msg = testMsgWithSize(ctrl, config.VdrAllocSize/2-1)
throttlerIntf.Release(msg, vdr1ID)
require.Len(throttler.nodeToVdrBytesUsed, 0)
require.Empty(throttler.nodeToVdrBytesUsed)
require.Equal(config.VdrAllocSize, throttler.remainingVdrBytes)
require.Equal(config.AtLargeAllocSize/2-1, throttler.remainingAtLargeBytes)
require.Zero(throttler.nodeToAtLargeBytesUsed[vdr1ID])

// Release nonVdr's messages
msg = testMsgWithSize(ctrl, config.AtLargeAllocSize/2+1)
throttlerIntf.Release(msg, nonVdrID)
require.Len(throttler.nodeToVdrBytesUsed, 0)
require.Empty(throttler.nodeToVdrBytesUsed)
require.Equal(config.VdrAllocSize, throttler.remainingVdrBytes)
require.Equal(config.AtLargeAllocSize, throttler.remainingAtLargeBytes)
require.Len(throttler.nodeToAtLargeBytesUsed, 0)
require.Empty(throttler.nodeToAtLargeBytesUsed)
require.Zero(throttler.nodeToAtLargeBytesUsed[nonVdrID])
}

Expand Down
11 changes: 10 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"}
TESTS=${TESTS:-"golangci_lint license_header require_error_is_no_funcs_as_params single_import interface_compliance_nil require_equal_zero require_len_zero"}

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

function test_require_len_zero {
if grep -R -o -P 'require\.Len\((t, )?.+, 0(,|\))' .; then
echo ""
echo "Use require.Empty instead of require.Len when testing for 0 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
34 changes: 17 additions & 17 deletions snow/consensus/snowman/poll/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,17 @@ func TestCreateAndFinishPollOutOfOrder_NewerFinishesFirst(t *testing.T) {

// vote out of order
results = s.Vote(1, vdr1, vtx1)
require.Len(t, results, 0)
require.Empty(t, results)
results = s.Vote(2, vdr2, vtx2)
require.Len(t, results, 0)
require.Empty(t, results)
results = s.Vote(2, vdr3, vtx2)
require.Len(t, results, 0)
require.Empty(t, results)

results = s.Vote(2, vdr1, vtx2) // poll 2 finished
require.Len(t, results, 0) // expect 2 to not have finished because 1 is still pending
require.Empty(t, results) // expect 2 to not have finished because 1 is still pending

results = s.Vote(1, vdr2, vtx1)
require.Len(t, results, 0)
require.Empty(t, results)

results = s.Vote(1, vdr3, vtx1) // poll 1 finished, poll 2 should be finished as well
require.Len(t, results, 2)
Expand Down Expand Up @@ -129,14 +129,14 @@ func TestCreateAndFinishPollOutOfOrder_OlderFinishesFirst(t *testing.T) {

// vote out of order
results = s.Vote(1, vdr1, vtx1)
require.Len(t, results, 0)
require.Empty(t, results)
results = s.Vote(2, vdr2, vtx2)
require.Len(t, results, 0)
require.Empty(t, results)
results = s.Vote(2, vdr3, vtx2)
require.Len(t, results, 0)
require.Empty(t, results)

results = s.Vote(1, vdr2, vtx1)
require.Len(t, results, 0)
require.Empty(t, results)

results = s.Vote(1, vdr3, vtx1) // poll 1 finished, poll 2 still remaining
require.Len(t, results, 1) // because 1 is the oldest
Expand Down Expand Up @@ -190,25 +190,25 @@ func TestCreateAndFinishPollOutOfOrder_UnfinishedPollsGaps(t *testing.T) {
// vote out of order
// 2 finishes first to create a gap of finished poll between two unfinished polls 1 and 3
results = s.Vote(2, vdr3, vtx2)
require.Len(t, results, 0)
require.Empty(t, results)
results = s.Vote(2, vdr2, vtx2)
require.Len(t, results, 0)
require.Empty(t, results)
results = s.Vote(2, vdr1, vtx2)
require.Len(t, results, 0)
require.Empty(t, results)

// 3 finishes now, 2 has already finished but 1 is not finished so we expect to receive no results still
results = s.Vote(3, vdr2, vtx3)
require.Len(t, results, 0)
require.Empty(t, results)
results = s.Vote(3, vdr3, vtx3)
require.Len(t, results, 0)
require.Empty(t, results)
results = s.Vote(3, vdr1, vtx3)
require.Len(t, results, 0)
require.Empty(t, results)

// 1 finishes now, 2 and 3 have already finished so we expect 3 items in results
results = s.Vote(1, vdr1, vtx1)
require.Len(t, results, 0)
require.Empty(t, results)
results = s.Vote(1, vdr2, vtx1)
require.Len(t, results, 0)
require.Empty(t, results)
results = s.Vote(1, vdr3, vtx1)
require.Len(t, results, 3)
require.Equal(t, vtx1, results[0].List()[0])
Expand Down
2 changes: 1 addition & 1 deletion snow/engine/snowman/block/batched_vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestGetAncestorsDatabaseNotFound(t *testing.T) {
}
containers, err := GetAncestors(context.Background(), vm, someID, 10, 10, 1*time.Second)
require.NoError(t, err)
require.Len(t, containers, 0)
require.Empty(t, containers)
}

// TestGetAncestorsPropagatesErrors checks errors other than
Expand Down
10 changes: 5 additions & 5 deletions snow/networking/benchlist/benchlist_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestBenchlistAdd(t *testing.T) {
require.False(t, b.isBenched(vdrID2))
require.False(t, b.isBenched(vdrID3))
require.False(t, b.isBenched(vdrID4))
require.Len(t, b.failureStreaks, 0)
require.Empty(t, b.failureStreaks)
require.Zero(t, b.benchedQueue.Len())
require.Zero(t, b.benchlistSet.Len())
b.lock.Unlock()
Expand Down Expand Up @@ -126,7 +126,7 @@ func TestBenchlistAdd(t *testing.T) {
require.Equal(t, vdrID0, next.nodeID)
require.True(t, !next.benchedUntil.After(now.Add(duration)))
require.True(t, !next.benchedUntil.Before(now.Add(duration/2)))
require.Len(t, b.failureStreaks, 0)
require.Empty(t, b.failureStreaks)
require.True(t, benched)
benchable.BenchedF = nil
b.lock.Unlock()
Expand All @@ -146,15 +146,15 @@ func TestBenchlistAdd(t *testing.T) {
require.False(t, b.isBenched(vdrID1))
require.Equal(t, b.benchedQueue.Len(), 1)
require.Equal(t, b.benchlistSet.Len(), 1)
require.Len(t, b.failureStreaks, 0)
require.Empty(t, b.failureStreaks)
b.lock.Unlock()

// Register another failure for vdr0, who is benched
b.RegisterFailure(vdrID0)

// A failure for an already benched validator should not count against it
b.lock.Lock()
require.Len(t, b.failureStreaks, 0)
require.Empty(t, b.failureStreaks)
b.lock.Unlock()
}

Expand Down Expand Up @@ -369,7 +369,7 @@ func TestBenchlistRemove(t *testing.T) {
require.True(t, b.isBenched(vdrID2))
require.Equal(t, 3, b.benchedQueue.Len())
require.Equal(t, 3, b.benchlistSet.Len())
require.Len(t, b.failureStreaks, 0)
require.Empty(t, b.failureStreaks)

// Ensure the benched queue root has the min end time
minEndTime := b.benchedQueue[0].benchedUntil
Expand Down
Loading

0 comments on commit d146232

Please sign in to comment.