Skip to content

ban usage of require.Len when testing for length 0 #1496

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

Merged
merged 18 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
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