mark all test helper funcs via t.Helper (#276)

This will allow testing errors to point to the caller position,
which is more useful than where the test helper is declared.

While here, use testing.TB for its intended purpose.
That interface is broader than TestingT,
but we're also never going to use an implementation other than testing.

allocator/allocator_test.go

@@ -395,6 +395,7 @@ func TestAllocator(t *testing.T) {
 }
 
 func readPending(t *testing.T, pending []pendingResultWithChan) []pendingResultWithChan {
+	t.Helper()
 	morePending := true
 	for morePending && len(pending) > 0 {
 		morePending = false

impl/graphsync_test.go

@@ -1145,6 +1145,7 @@ type gsTestData struct {
 }
 
 func newGsTestData(ctx context.Context, t *testing.T) *gsTestData {
+	t.Helper()
 	td := &gsTestData{ctx: ctx}
 	td.mn = mocknet.New(ctx)
 	var err error

ipldutil/traverser_test.go

@@ -140,6 +140,7 @@ func TestTraverser(t *testing.T) {
 }
 
 func checkTraverseSequence(ctx context.Context, t *testing.T, traverser Traverser, expectedBlks []blocks.Block, finalErr error) {
+	t.Helper()
 	for _, blk := range expectedBlks {
 		isComplete, err := traverser.IsComplete()
 		require.False(t, isComplete)

requestmanager/asyncloader/asyncloader_test.go

@@ -391,13 +391,15 @@ func withLoader(st *store, exec func(ctx context.Context, asyncLoader *AsyncLoad
 }
 
 func assertSuccessResponse(ctx context.Context, t *testing.T, resultChan <-chan types.AsyncLoadResult) {
+	t.Helper()
 	var result types.AsyncLoadResult
 	testutil.AssertReceive(ctx, t, resultChan, &result, "should close response channel with response")
 	require.NotNil(t, result.Data, "should send response")
 	require.Nil(t, result.Err, "should not send error")
 }
 
 func assertFailResponse(ctx context.Context, t *testing.T, resultChan <-chan types.AsyncLoadResult) {
+	t.Helper()
 	var result types.AsyncLoadResult
 	testutil.AssertReceive(ctx, t, resultChan, &result, "should close response channel with response")
 	require.Nil(t, result.Data, "should not send responses")

requestmanager/requestmanager_test.go

@@ -1031,6 +1031,7 @@ func metadataForBlocks(blks []blocks.Block, present bool) metadata.Metadata {
 }
 
 func encodedMetadataForBlocks(t *testing.T, blks []blocks.Block, present bool) graphsync.ExtensionData {
+	t.Helper()
 	md := metadataForBlocks(blks, present)
 	metadataEncoded, err := metadata.EncodeMetadata(md)
 	require.NoError(t, err, "did not encode metadata")
@@ -1065,6 +1066,7 @@ type testData struct {
 }
 
 func newTestData(ctx context.Context, t *testing.T) *testData {
+	t.Helper()
 	td := &testData{}
 	td.requestRecordChan = make(chan requestRecord, 3)
 	td.fph = &fakePeerHandler{td.requestRecordChan}

requestmanager/testloader/asyncloader.go

@@ -70,6 +70,7 @@ func (fal *FakeAsyncLoader) ProcessResponse(responses map[graphsync.RequestID]me
 // VerifyLastProcessedBlocks verifies the blocks passed to the last call to ProcessResponse
 // match the expected ones
 func (fal *FakeAsyncLoader) VerifyLastProcessedBlocks(ctx context.Context, t *testing.T, expectedBlocks []blocks.Block) {
+	t.Helper()
 	var processedBlocks []blocks.Block
 	testutil.AssertReceive(ctx, t, fal.blks, &processedBlocks, "did not process blocks")
 	require.Equal(t, expectedBlocks, processedBlocks, "did not process correct blocks")
@@ -79,6 +80,7 @@ func (fal *FakeAsyncLoader) VerifyLastProcessedBlocks(ctx context.Context, t *te
 // match the expected ones
 func (fal *FakeAsyncLoader) VerifyLastProcessedResponses(ctx context.Context, t *testing.T,
 	expectedResponses map[graphsync.RequestID]metadata.Metadata) {
+	t.Helper()
 	var responses map[graphsync.RequestID]metadata.Metadata
 	testutil.AssertReceive(ctx, t, fal.responses, &responses, "did not process responses")
 	require.Equal(t, expectedResponses, responses, "did not process correct responses")
@@ -87,6 +89,7 @@ func (fal *FakeAsyncLoader) VerifyLastProcessedResponses(ctx context.Context, t
 // VerifyNoRemainingData verifies no outstanding response channels are open for the given
 // RequestID (CleanupRequest was called last)
 func (fal *FakeAsyncLoader) VerifyNoRemainingData(t *testing.T, requestID graphsync.RequestID) {
+	t.Helper()
 	fal.responseChannelsLk.RLock()
 	for key := range fal.responseChannels {
 		require.NotEqual(t, key.requestID, requestID, "did not clean up request properly")
@@ -96,6 +99,7 @@ func (fal *FakeAsyncLoader) VerifyNoRemainingData(t *testing.T, requestID graphs
 
 // VerifyStoreUsed verifies the given store was used for the given request
 func (fal *FakeAsyncLoader) VerifyStoreUsed(t *testing.T, requestID graphsync.RequestID, storeName string) {
+	t.Helper()
 	fal.storesRequestedLk.RLock()
 	_, ok := fal.storesRequested[storeKey{requestID, storeName}]
 	require.True(t, ok, "request should load from correct store")

responsemanager/queryexecutor/queryexecutor_test.go

@@ -243,6 +243,7 @@ func TestSmallGraphTask(t *testing.T) {
 }
 
 func notifeeExpect(t *testing.T, td *testData, expectedCalls int, expectedFinalData notifications.TopicData) {
+	t.Helper()
 	notifeeCount := 1
 	td.responseBuilder.notifeeCb = func(n notifications.Notifee) {
 		require.Same(t, td.subscriber, n.Subscriber)
@@ -307,6 +308,7 @@ type testData struct {
 }
 
 func newTestData(t *testing.T, blockCount int, expectedTraverse int) (*testData, *QueryExecutor) {
+	t.Helper()
 	ctx := context.Background()
 	td := &testData{}
 	td.t = t

responsemanager/responseassembler/responseassembler_test.go

@@ -424,18 +424,21 @@ func findResponseForRequestID(responses []gsmsg.GraphSyncResponse, requestID gra
 }
 
 func assertSentNotOnWire(t *testing.T, bd graphsync.BlockData, blk blocks.Block) {
+	t.Helper()
 	require.Equal(t, cidlink.Link{Cid: blk.Cid()}, bd.Link())
 	require.Equal(t, uint64(len(blk.RawData())), bd.BlockSize())
 	require.Equal(t, uint64(0), bd.BlockSizeOnWire())
 }
 
 func assertSentOnWire(t *testing.T, bd graphsync.BlockData, blk blocks.Block) {
+	t.Helper()
 	require.Equal(t, cidlink.Link{Cid: blk.Cid()}, bd.Link())
 	require.Equal(t, uint64(len(blk.RawData())), bd.BlockSize())
 	require.Equal(t, uint64(len(blk.RawData())), bd.BlockSizeOnWire())
 }
 
 func assertNotSent(t *testing.T, bd graphsync.BlockData, blk blocks.Block) {
+	t.Helper()
 	require.Equal(t, cidlink.Link{Cid: blk.Cid()}, bd.Link())
 	require.Equal(t, uint64(0), bd.BlockSize())
 	require.Equal(t, uint64(0), bd.BlockSizeOnWire())
@@ -451,6 +454,7 @@ type fakePeerHandler struct {
 }
 
 func newFakePeerHandler(ctx context.Context, t *testing.T) *fakePeerHandler {
+	t.Helper()
 	return &fakePeerHandler{
 		ctx: ctx,
 		t:   t,

responsemanager/responsemanager_test.go

@@ -989,6 +989,7 @@ type testData struct {
 }
 
 func newTestData(t *testing.T) testData {
+	t.Helper()
 	ctx := context.Background()
 	td := testData{}
 	td.t = t

testutil/channelassertions.go

@@ -3,19 +3,22 @@ package testutil
 import (
 	"context"
 	"reflect"
+	"testing"
 
 	"github.com/stretchr/testify/require"
 )
 
 // AssertReceive verifies that a channel returns a value before the given context closes, and writes into
 // into out, which should be a pointer to the value type
-func AssertReceive(ctx context.Context, t TestingT, channel interface{}, out interface{}, errorMessage string) {
+func AssertReceive(ctx context.Context, t testing.TB, channel interface{}, out interface{}, errorMessage string) {
+	t.Helper()
 	AssertReceiveFirst(t, channel, out, errorMessage, ctx.Done())
 }
 
 // AssertReceiveFirst verifies that a channel returns a value on the specified channel before the other channels,
 // and writes the value into out, which should be a pointer to the value type
-func AssertReceiveFirst(t TestingT, channel interface{}, out interface{}, errorMessage string, incorrectChannels ...interface{}) {
+func AssertReceiveFirst(t testing.TB, channel interface{}, out interface{}, errorMessage string, incorrectChannels ...interface{}) {
+	t.Helper()
 	chanValue := reflect.ValueOf(channel)
 	outValue := reflect.ValueOf(out)
 	require.Equal(t, reflect.Chan, chanValue.Kind(), "incorrect argument: should pass channel to read from")
@@ -44,12 +47,14 @@ func AssertReceiveFirst(t TestingT, channel interface{}, out interface{}, errorM
 }
 
 // AssertDoesReceive verifies that a channel returns some value before the given context closes
-func AssertDoesReceive(ctx context.Context, t TestingT, channel interface{}, errorMessage string) {
+func AssertDoesReceive(ctx context.Context, t testing.TB, channel interface{}, errorMessage string) {
+	t.Helper()
 	AssertDoesReceiveFirst(t, channel, errorMessage, ctx.Done())
 }
 
 // AssertDoesReceiveFirst asserts that the given channel receives a value before any of the other channels specified
-func AssertDoesReceiveFirst(t TestingT, channel interface{}, errorMessage string, incorrectChannels ...interface{}) {
+func AssertDoesReceiveFirst(t testing.TB, channel interface{}, errorMessage string, incorrectChannels ...interface{}) {
+	t.Helper()
 	chanValue := reflect.ValueOf(channel)
 	require.Equal(t, reflect.Chan, chanValue.Kind(), "incorrect argument: should pass channel to read from")
 	require.Contains(t, []reflect.ChanDir{reflect.BothDir, reflect.RecvDir}, chanValue.Type().ChanDir(), "incorrect argument: should pass a receiving channel")
@@ -73,7 +78,8 @@ func AssertDoesReceiveFirst(t TestingT, channel interface{}, errorMessage string
 }
 
 // AssertChannelEmpty verifies that a channel has no value currently
-func AssertChannelEmpty(t TestingT, channel interface{}, errorMessage string) {
+func AssertChannelEmpty(t testing.TB, channel interface{}, errorMessage string) {
+	t.Helper()
 	chanValue := reflect.ValueOf(channel)
 	require.Equal(t, reflect.Chan, chanValue.Kind(), "incorrect argument: should pass channel to read from")
 	require.Contains(t, []reflect.ChanDir{reflect.BothDir, reflect.RecvDir}, chanValue.Type().ChanDir(), "incorrect argument: should pass a receiving channel")
@@ -90,7 +96,8 @@ func AssertChannelEmpty(t TestingT, channel interface{}, errorMessage string) {
 }
 
 // AssertSends attempts to send the given input value to the given channel before the given context closes
-func AssertSends(ctx context.Context, t TestingT, channel interface{}, in interface{}, errorMessage string) {
+func AssertSends(ctx context.Context, t testing.TB, channel interface{}, in interface{}, errorMessage string) {
+	t.Helper()
 	chanValue := reflect.ValueOf(channel)
 	inValue := reflect.ValueOf(in)
 	require.Equal(t, reflect.Chan, chanValue.Kind(), "incorrect argument: should pass channel to send to")

testutil/testchain.go

@@ -3,6 +3,7 @@ package testutil
 import (
 	"context"
 	"io/ioutil"
+	"testing"
 
 	blocks "github.com/ipfs/go-block-format"
 	cid "github.com/ipfs/go-cid"
@@ -18,20 +19,12 @@ import (
 	"github.com/ipfs/go-graphsync/testutil/chaintypes"
 )
 
-// TestingT covers the interface methods we need from either *testing.T or
-// *testing.B
-type TestingT interface {
-	Errorf(format string, args ...interface{})
-	FailNow()
-	Fatal(args ...interface{})
-}
-
 const blockChainTraversedNodesPerBlock = 2
 
 // TestBlockChain is a simulated data structure similar to a blockchain
 // which graphsync is uniquely suited for
 type TestBlockChain struct {
-	t                TestingT
+	t                testing.TB
 	blockChainLength int
 	loader           ipld.BlockReadOpener
 	GenisisNode      ipld.Node
@@ -95,7 +88,7 @@ func createBlock(parents []ipld.Link, size uint64) (ipld.Node, error) {
 // SetupBlockChain creates a new test block chain with the given height
 func SetupBlockChain(
 	ctx context.Context,
-	t TestingT,
+	t testing.TB,
 	lsys ipld.LinkSystem,
 	size uint64,
 	blockChainLength int) *TestBlockChain {

testutil/testconnmanager.go

@@ -2,6 +2,7 @@ package testutil
 
 import (
 	"sync"
+	"testing"
 
 	"github.com/libp2p/go-libp2p-core/peer"
 	"github.com/stretchr/testify/require"
@@ -45,22 +46,25 @@ func (tcm *TestConnManager) Unprotect(p peer.ID, tag string) bool {
 }
 
 // AssertProtected asserts that the connection is protected by at least one tag
-func (tcm *TestConnManager) AssertProtected(t TestingT, p peer.ID) {
+func (tcm *TestConnManager) AssertProtected(t testing.TB, p peer.ID) {
+	t.Helper()
 	tcm.protectedConnsLk.RLock()
 	defer tcm.protectedConnsLk.RUnlock()
 	require.True(t, len(tcm.protectedConns[p]) > 0)
 }
 
 // RefuteProtected refutes that a connection has been protect
-func (tcm *TestConnManager) RefuteProtected(t TestingT, p peer.ID) {
+func (tcm *TestConnManager) RefuteProtected(t testing.TB, p peer.ID) {
+	t.Helper()
 	tcm.protectedConnsLk.RLock()
 	defer tcm.protectedConnsLk.RUnlock()
 	require.False(t, len(tcm.protectedConns[p]) > 0)
 }
 
 // AssertProtectedWithTags verifies the connection is protected with the given
 // tags at least
-func (tcm *TestConnManager) AssertProtectedWithTags(t TestingT, p peer.ID, tags ...string) {
+func (tcm *TestConnManager) AssertProtectedWithTags(t testing.TB, p peer.ID, tags ...string) {
+	t.Helper()
 	tcm.protectedConnsLk.RLock()
 	defer tcm.protectedConnsLk.RUnlock()
 	for _, tag := range tags {
@@ -70,7 +74,8 @@ func (tcm *TestConnManager) AssertProtectedWithTags(t TestingT, p peer.ID, tags
 
 // RefuteProtectedWithTags verifies the connection is not protected with any of the given
 // tags
-func (tcm *TestConnManager) RefuteProtectedWithTags(t TestingT, p peer.ID, tags ...string) {
+func (tcm *TestConnManager) RefuteProtectedWithTags(t testing.TB, p peer.ID, tags ...string) {
+	t.Helper()
 	tcm.protectedConnsLk.RLock()
 	defer tcm.protectedConnsLk.RUnlock()
 	for _, tag := range tags {

testutil/testnotifications.go

@@ -36,6 +36,7 @@ func (ts *TestSubscriber) OnClose(topic notifications.Topic) {
 }
 
 func (ts *TestSubscriber) ExpectEvents(ctx context.Context, t *testing.T, events []DispatchedEvent) {
+	t.Helper()
 	for _, expectedEvent := range events {
 		var event DispatchedEvent
 		AssertReceive(ctx, t, ts.receivedEvents, &event, "should receive another event")
@@ -44,10 +45,12 @@ func (ts *TestSubscriber) ExpectEvents(ctx context.Context, t *testing.T, events
 }
 
 func (ts *TestSubscriber) NoEventsReceived(t *testing.T) {
+	t.Helper()
 	AssertChannelEmpty(t, ts.receivedEvents, "should have received no events")
 }
 
 func (ts *TestSubscriber) ExpectClosesAnyOrder(ctx context.Context, t *testing.T, topics []notifications.Topic) {
+	t.Helper()
 	expectedTopics := make(map[notifications.Topic]struct{})
 	receivedTopics := make(map[notifications.Topic]struct{})
 	for _, expectedTopic := range topics {
@@ -60,6 +63,7 @@ func (ts *TestSubscriber) ExpectClosesAnyOrder(ctx context.Context, t *testing.T
 }
 
 func (ts *TestSubscriber) ExpectCloses(ctx context.Context, t *testing.T, topics []notifications.Topic) {
+	t.Helper()
 	for _, expectedTopic := range topics {
 		var topic notifications.Topic
 		AssertReceive(ctx, t, ts.closed, &topic, "should receive another event")
@@ -73,6 +77,7 @@ type NotifeeVerifier struct {
 }
 
 func (nv *NotifeeVerifier) ExpectEvents(ctx context.Context, t *testing.T, events []notifications.Event) {
+	t.Helper()
 	dispatchedEvents := make([]DispatchedEvent, 0, len(events))
 	for _, ev := range events {
 		dispatchedEvents = append(dispatchedEvents, DispatchedEvent{nv.expectedTopic, ev})
@@ -81,6 +86,7 @@ func (nv *NotifeeVerifier) ExpectEvents(ctx context.Context, t *testing.T, event
 }
 
 func (nv *NotifeeVerifier) ExpectClose(ctx context.Context, t *testing.T) {
+	t.Helper()
 	nv.subscriber.ExpectCloses(ctx, t, []notifications.Topic{nv.expectedTopic})
 }