From 1ac48243c0f8ea5291b8d5caf9c6207bb7ddfce4 Mon Sep 17 00:00:00 2001 From: Jorropo Date: Sat, 6 Aug 2022 23:13:33 +0200 Subject: [PATCH] test: remove TestTracer This test is exceptionally racy and IMO useless (you can go read the 10 lines of code making up tracing and convaince yourself it's working.) --- bitswap_test.go | 157 ------------------------------------------------ options.go | 11 +--- polyfill.go | 2 +- 3 files changed, 2 insertions(+), 168 deletions(-) diff --git a/bitswap_test.go b/bitswap_test.go index 7c32c646..33603726 100644 --- a/bitswap_test.go +++ b/bitswap_test.go @@ -12,10 +12,8 @@ import ( "github.com/ipfs/go-bitswap" testinstance "github.com/ipfs/go-bitswap/client/testinstance" bsmsg "github.com/ipfs/go-bitswap/message" - pb "github.com/ipfs/go-bitswap/message/pb" "github.com/ipfs/go-bitswap/server" tn "github.com/ipfs/go-bitswap/testnet" - "github.com/ipfs/go-bitswap/tracer" blocks "github.com/ipfs/go-block-format" cid "github.com/ipfs/go-cid" detectrace "github.com/ipfs/go-detect-race" @@ -830,158 +828,3 @@ func TestWithScoreLedger(t *testing.T) { t.Fatal("Expected the score ledger to be closed within 5s") } } - -type logItem struct { - dir byte - pid peer.ID - msg bsmsg.BitSwapMessage -} -type mockTracer struct { - mu sync.Mutex - log []logItem -} - -func (m *mockTracer) MessageReceived(p peer.ID, msg bsmsg.BitSwapMessage) { - m.mu.Lock() - defer m.mu.Unlock() - m.log = append(m.log, logItem{'r', p, msg}) -} -func (m *mockTracer) MessageSent(p peer.ID, msg bsmsg.BitSwapMessage) { - m.mu.Lock() - defer m.mu.Unlock() - m.log = append(m.log, logItem{'s', p, msg}) -} - -func (m *mockTracer) getLog() []logItem { - m.mu.Lock() - defer m.mu.Unlock() - return m.log[:len(m.log):len(m.log)] -} - -func TestTracer(t *testing.T) { - net := tn.VirtualNetwork(mockrouting.NewServer(), delay.Fixed(kNetworkDelay)) - ig := testinstance.NewTestInstanceGenerator(net, nil, nil) - defer ig.Close() - bg := blocksutil.NewBlockGenerator() - - instances := ig.Instances(3) - blocks := bg.Blocks(2) - - // Install Tracer - wiretap := new(mockTracer) - updateTracer(instances[0].Exchange, wiretap) - - // First peer has block - addBlock(t, context.Background(), instances[0], blocks[0]) - - ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) - defer cancel() - - // Second peer broadcasts want for block CID - // (Received by first and third peers) - _, err := instances[1].Exchange.GetBlock(ctx, blocks[0].Cid()) - if err != nil { - t.Fatal(err) - } - - // When second peer receives block, it should send out a cancel, so third - // peer should no longer keep second peer's want - if err = tu.WaitFor(ctx, func() error { - if len(instances[2].Exchange.WantlistForPeer(instances[1].Peer)) != 0 { - return fmt.Errorf("should have no items in other peers wantlist") - } - if len(instances[1].Exchange.GetWantlist()) != 0 { - return fmt.Errorf("shouldnt have anything in wantlist") - } - return nil - }); err != nil { - t.Fatal(err) - } - - log := wiretap.getLog() - - // After communication, 3 messages should be logged via Tracer - if l := len(log); l != 3 { - t.Fatal("expected 3 items logged via Tracer, found", l) - } - - // Received: 'Have' - if log[0].dir != 'r' { - t.Error("expected message to be received") - } - if log[0].pid != instances[1].Peer { - t.Error("expected peer", instances[1].Peer, ", found", log[0].pid) - } - if l := len(log[0].msg.Wantlist()); l != 1 { - t.Fatal("expected 1 entry in Wantlist, found", l) - } - if log[0].msg.Wantlist()[0].WantType != pb.Message_Wantlist_Have { - t.Error("expected WantType equal to 'Have', found 'Block'") - } - - // Sent: Block - if log[1].dir != 's' { - t.Error("expected message to be sent") - } - if log[1].pid != instances[1].Peer { - t.Error("expected peer", instances[1].Peer, ", found", log[1].pid) - } - if l := len(log[1].msg.Blocks()); l != 1 { - t.Fatal("expected 1 entry in Blocks, found", l) - } - if log[1].msg.Blocks()[0].Cid() != blocks[0].Cid() { - t.Error("wrong block Cid") - } - - // Received: 'Cancel' - if log[2].dir != 'r' { - t.Error("expected message to be received") - } - if log[2].pid != instances[1].Peer { - t.Error("expected peer", instances[1].Peer, ", found", log[2].pid) - } - if l := len(log[2].msg.Wantlist()); l != 1 { - t.Fatal("expected 1 entry in Wantlist, found", l) - } - if log[2].msg.Wantlist()[0].WantType != pb.Message_Wantlist_Block { - t.Error("expected WantType equal to 'Block', found 'Have'") - } - if log[2].msg.Wantlist()[0].Cancel != true { - t.Error("expected entry with Cancel set to 'true'") - } - - // After disabling WireTap, no new messages are logged - updateTracer(instances[0].Exchange, nil) - - addBlock(t, context.Background(), instances[0], blocks[1]) - - _, err = instances[1].Exchange.GetBlock(ctx, blocks[1].Cid()) - if err != nil { - t.Fatal(err) - } - if err = tu.WaitFor(ctx, func() error { - if len(instances[1].Exchange.GetWantlist()) != 0 { - return fmt.Errorf("shouldnt have anything in wantlist") - } - return nil - }); err != nil { - t.Fatal(err) - } - - log = wiretap.getLog() - - if l := len(log); l != 3 { - t.Fatal("expected 3 items logged via WireTap, found", l) - } - - for _, inst := range instances { - err := inst.Exchange.Close() - if err != nil { - t.Fatal(err) - } - } -} - -func updateTracer(bs *bitswap.Bitswap, tap tracer.Tracer) { - bitswap.WithTracer(tap).V.(func(*bitswap.Bitswap))(bs) -} diff --git a/options.go b/options.go index 0c087b71..934396a7 100644 --- a/options.go +++ b/options.go @@ -14,7 +14,7 @@ type option func(*Bitswap) // Option is interface{} of server.Option or client.Option or func(*Bitswap) // wrapped in a struct to gain strong type checking. type Option struct { - V interface{} + v interface{} } func EngineBlockstoreWorkerCount(count int) Option { @@ -74,15 +74,6 @@ func WithTracer(tap tracer.Tracer) Option { return Option{ func(bs *Bitswap) { bs.tracer = tap - // the tests use this to hot update tracers, we need to update tracers of impls if we are running - if bs.Client != nil { - if tap != nil { - tap = nopReceiveTracer{tap} - } - client.WithTracer(tap)(bs.Client) - // no need to check for server as they can't not be both running - server.WithTracer(tap)(bs.Server) - } }, } } diff --git a/polyfill.go b/polyfill.go index 3ca47b1b..95dcd5dc 100644 --- a/polyfill.go +++ b/polyfill.go @@ -63,7 +63,7 @@ func New(ctx context.Context, net network.BitSwapNetwork, bstore blockstore.Bloc var clientOptions []client.Option for _, o := range options { - switch typedOption := o.V.(type) { + switch typedOption := o.v.(type) { case server.Option: serverOptions = append(serverOptions, typedOption) case client.Option: