Fix beacon-chain/sync Test Race

.travis-bazelrc

@@ -12,3 +12,4 @@ build --verbose_failures
 build --sandbox_debug
 build --test_output=errors
 build --flaky_test_attempts=5
+build --features=race

beacon-chain/sync/service_test.go

@@ -785,22 +785,26 @@ func TestSavingBlocksInSync(t *testing.T) {
 		t.Fatalf("unable to get hash of crystallized state: %v", err)
 	}
 
-	block := &pb.BeaconBlock{
-		MainChainRef:          []byte{1, 2, 3},
-		ParentHash:            generichash,
-		SlotNumber:            uint64(20),
-		CrystallizedStateHash: crystallizedStateHash[:],
-	}
+	getBlockResponseMsg := func(slotNumber uint64) p2p.Message {
+		block := &pb.BeaconBlock{
+			MainChainRef:          []byte{1, 2, 3},
+			ParentHash:            generichash,
+			SlotNumber:            slotNumber,
+			CrystallizedStateHash: crystallizedStateHash[:],
+		}
 
-	blockResponse := &pb.BeaconBlockResponse{
-		Block: block,
-	}
+		blockResponse := &pb.BeaconBlockResponse{
+			Block: block,
+		}
 
-	msg1 := p2p.Message{
-		Peer: p2p.Peer{},
-		Data: blockResponse,
+		return p2p.Message{
+			Peer: p2p.Peer{},
+			Data: blockResponse,
+		}
 	}
 
+	msg1 := getBlockResponseMsg(0)
+
 	msg2 := p2p.Message{
 		Peer: p2p.Peer{},
 		Data: incorrectStateResponse,
@@ -818,24 +822,25 @@ func TestSavingBlocksInSync(t *testing.T) {
 	ss.crystallizedStateBuf <- msg2
 
 	if crystallizedStateHash != ss.initialCrystallizedStateHash {
-		t.Fatalf("Crystallized state hash not updated: %x", blockResponse.Block.CrystallizedStateHash)
+		br := msg1.Data.(*pb.BeaconBlockResponse)
+		t.Fatalf("Crystallized state hash not updated: %x", br.Block.CrystallizedStateHash)
 	}
 
-	blockResponse.Block.SlotNumber = 30
-	msg1.Data = blockResponse
+	msg1 = getBlockResponseMsg(30)
 	ss.blockBuf <- msg1
 
 	if stateResponse.CrystallizedState.GetLastFinalizedEpoch() != ss.currentSlotNumber {
 		t.Fatalf("slotnumber saved when it was not supposed too: %v", stateResponse.CrystallizedState.GetLastFinalizedEpoch())
 	}
 
-	blockResponse.Block.SlotNumber = 100
+	msg1 = getBlockResponseMsg(100)
 	ss.blockBuf <- msg1
 
 	ss.cancel()
 	<-exitRoutine
 
-	if blockResponse.Block.GetSlotNumber() != ss.currentSlotNumber {
+	br := msg1.Data.(*pb.BeaconBlockResponse)
+	if br.Block.GetSlotNumber() != ss.currentSlotNumber {
 		t.Fatalf("slotnumber not updated despite receiving a valid block: %v", ss.currentSlotNumber)
 	}
 

client/contracts/BUILD.bazel

@@ -23,6 +23,7 @@ go_test(
     name = "go_default_test",
     srcs = ["sharding_manager_test.go"],
     embed = [":go_default_library"],
+    race = "off",
     deps = [
         "//client/params:go_default_library",
         "@com_github_ethereum_go_ethereum//accounts/abi/bind:go_default_library",

client/syncer/BUILD.bazel

@@ -45,6 +45,29 @@ go_test(
         "@com_github_ethereum_go_ethereum//core:go_default_library",
         "@com_github_ethereum_go_ethereum//core/types:go_default_library",
         "@com_github_ethereum_go_ethereum//crypto:go_default_library",
+        "@com_github_sirupsen_logrus//hooks/test:go_default_library",
+    ],
+)
+
+# by default gazelle tries to add all the test files to the  first
+# go_test; we need to exclude the tests that we want to configure
+# in a particular way
+# gazelle:exclude service_norace_test.go
+
+go_test(
+    name = "go_norace_test",
+    srcs = ["service_norace_test.go"],
+    embed = [":go_default_library"],
+    race = "off",  # TODO(#377): fix issues with race detection testing.
+    deps = [
+        "//client/mainchain:go_default_library",
+        "//client/params:go_default_library",
+        "//client/types:go_default_library",
+        "//proto/sharding/p2p/v1:go_default_library",
+        "//shared/database:go_default_library",
+        "//shared/p2p:go_default_library",
+        "@com_github_ethereum_go_ethereum//common:go_default_library",
+        "@com_github_ethereum_go_ethereum//core/types:go_default_library",
         "@com_github_sirupsen_logrus//:go_default_library",
         "@com_github_sirupsen_logrus//hooks/test:go_default_library",
     ],

client/syncer/service_norace_test.go

@@ -0,0 +1,98 @@
+package syncer
+
+import (
+	"fmt"
+	"io/ioutil"
+	"math/big"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	gethTypes "github.com/ethereum/go-ethereum/core/types"
+	"github.com/prysmaticlabs/prysm/client/mainchain"
+	"github.com/prysmaticlabs/prysm/client/params"
+	"github.com/prysmaticlabs/prysm/client/types"
+	pb "github.com/prysmaticlabs/prysm/proto/sharding/p2p/v1"
+	"github.com/prysmaticlabs/prysm/shared/database"
+	"github.com/prysmaticlabs/prysm/shared/p2p"
+	"github.com/sirupsen/logrus"
+	logTest "github.com/sirupsen/logrus/hooks/test"
+)
+
+func init() {
+	logrus.SetLevel(logrus.DebugLevel)
+	logrus.SetOutput(ioutil.Discard)
+}
+
+// This test checks the proper functioning of the handleCollationBodyRequests goroutine
+// by listening to the responseSent channel which occurs after successful
+// construction and sending of a response via p2p.
+func TestHandleCollationBodyRequests(t *testing.T) {
+	hook := logTest.NewGlobal()
+
+	config := &database.DBConfig{Name: "", DataDir: "", InMemory: true}
+	shardChainDB, err := database.NewDB(config)
+	if err != nil {
+		t.Fatalf("unable to setup db: %v", err)
+	}
+	server, err := p2p.NewServer()
+	if err != nil {
+		t.Fatalf("Unable to setup p2p server: %v", err)
+	}
+
+	body := []byte{1, 2, 3, 4, 5}
+	shardID := big.NewInt(0)
+	chunkRoot := gethTypes.DeriveSha(types.Chunks(body))
+	period := big.NewInt(0)
+	proposerAddress := common.BytesToAddress([]byte{})
+
+	header := types.NewCollationHeader(shardID, &chunkRoot, period, &proposerAddress, [32]byte{})
+	// Stores the collation into the inmemory kv store shardChainDB.
+	collation := types.NewCollation(header, body, nil)
+
+	shard := types.NewShard(shardID, shardChainDB.DB())
+
+	if err := shard.SaveCollation(collation); err != nil {
+		t.Fatalf("Could not store collation in shardChainDB: %v", err)
+	}
+
+	syncer, err := NewSyncer(params.DefaultConfig(), &mainchain.SMCClient{}, server, shardChainDB, 0)
+	if err != nil {
+		t.Fatalf("Unable to setup syncer service: %v", err)
+	}
+	syncer.Start()
+	syncer.collationBodyBuf = make(chan p2p.Message)
+
+	doneChan := make(chan struct{})
+	exitRoutine := make(chan bool)
+
+	go func() {
+		syncer.run(doneChan)
+		<-exitRoutine
+	}()
+
+	msg := p2p.Message{
+		Peer: p2p.Peer{},
+		Data: &pb.CollationBodyRequest{
+			ChunkRoot:       chunkRoot.Bytes(),
+			ShardId:         shardID.Uint64(),
+			Period:          period.Uint64(),
+			ProposerAddress: proposerAddress.Bytes(),
+		},
+	}
+	syncer.collationBodyBuf <- msg
+	doneChan <- struct{}{}
+	exitRoutine <- true
+
+	logMsg := hook.Entries[1].Message
+	want := fmt.Sprintf("Received p2p request of type: %T", &pb.CollationBodyRequest{})
+	if logMsg != want {
+		t.Errorf("incorrect log, expected %s, got %s", want, logMsg)
+	}
+
+	logMsg = hook.Entries[4].Message
+	want = fmt.Sprintf("Responding to p2p collation request")
+	if logMsg != want {
+		t.Errorf("incorrect log, expected %s, got %s", want, logMsg)
+	}
+	hook.Reset()
+}

client/syncer/service_test.go

@@ -1,31 +1,18 @@
 package syncer
 
 import (
-	"fmt"
-	"io/ioutil"
-	"math/big"
 	"testing"
 
-	"github.com/ethereum/go-ethereum/common"
-	gethTypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/prysmaticlabs/prysm/client/mainchain"
 	"github.com/prysmaticlabs/prysm/client/params"
-	"github.com/prysmaticlabs/prysm/client/types"
-	pb "github.com/prysmaticlabs/prysm/proto/sharding/p2p/v1"
 	"github.com/prysmaticlabs/prysm/shared"
 	"github.com/prysmaticlabs/prysm/shared/database"
 	"github.com/prysmaticlabs/prysm/shared/p2p"
-	"github.com/sirupsen/logrus"
 	logTest "github.com/sirupsen/logrus/hooks/test"
 )
 
 var _ = shared.Service(&Syncer{})
 
-func init() {
-	logrus.SetLevel(logrus.DebugLevel)
-	logrus.SetOutput(ioutil.Discard)
-}
-
 func TestStop(t *testing.T) {
 	hook := logTest.NewGlobal()
 
@@ -63,77 +50,3 @@ func TestStop(t *testing.T) {
 	}
 	hook.Reset()
 }
-
-// This test checks the proper functioning of the handleCollationBodyRequests goroutine
-// by listening to the responseSent channel which occurs after successful
-// construction and sending of a response via p2p.
-func TestHandleCollationBodyRequests(t *testing.T) {
-	hook := logTest.NewGlobal()
-
-	config := &database.DBConfig{Name: "", DataDir: "", InMemory: true}
-	shardChainDB, err := database.NewDB(config)
-	if err != nil {
-		t.Fatalf("unable to setup db: %v", err)
-	}
-	server, err := p2p.NewServer()
-	if err != nil {
-		t.Fatalf("Unable to setup p2p server: %v", err)
-	}
-
-	body := []byte{1, 2, 3, 4, 5}
-	shardID := big.NewInt(0)
-	chunkRoot := gethTypes.DeriveSha(types.Chunks(body))
-	period := big.NewInt(0)
-	proposerAddress := common.BytesToAddress([]byte{})
-
-	header := types.NewCollationHeader(shardID, &chunkRoot, period, &proposerAddress, [32]byte{})
-	// Stores the collation into the inmemory kv store shardChainDB.
-	collation := types.NewCollation(header, body, nil)
-
-	shard := types.NewShard(shardID, shardChainDB.DB())
-
-	if err := shard.SaveCollation(collation); err != nil {
-		t.Fatalf("Could not store collation in shardChainDB: %v", err)
-	}
-
-	syncer, err := NewSyncer(params.DefaultConfig(), &mainchain.SMCClient{}, server, shardChainDB, 0)
-	if err != nil {
-		t.Fatalf("Unable to setup syncer service: %v", err)
-	}
-	syncer.Start()
-	syncer.collationBodyBuf = make(chan p2p.Message)
-
-	doneChan := make(chan struct{})
-	exitRoutine := make(chan bool)
-
-	go func() {
-		syncer.run(doneChan)
-		<-exitRoutine
-	}()
-
-	msg := p2p.Message{
-		Peer: p2p.Peer{},
-		Data: &pb.CollationBodyRequest{
-			ChunkRoot:       chunkRoot.Bytes(),
-			ShardId:         shardID.Uint64(),
-			Period:          period.Uint64(),
-			ProposerAddress: proposerAddress.Bytes(),
-		},
-	}
-	syncer.collationBodyBuf <- msg
-	doneChan <- struct{}{}
-	exitRoutine <- true
-
-	logMsg := hook.Entries[1].Message
-	want := fmt.Sprintf("Received p2p request of type: %T", &pb.CollationBodyRequest{})
-	if logMsg != want {
-		t.Errorf("incorrect log, expected %s, got %s", want, logMsg)
-	}
-
-	logMsg = hook.Entries[4].Message
-	want = fmt.Sprintf("Responding to p2p collation request")
-	if logMsg != want {
-		t.Errorf("incorrect log, expected %s, got %s", want, logMsg)
-	}
-	hook.Reset()
-}

shared/p2p/BUILD.bazel

@@ -33,7 +33,6 @@ go_library(
 go_test(
     name = "go_default_test",
     srcs = [
-        "discovery_test.go",
         "feed_example_test.go",
         "feed_test.go",
         "options_test.go",
@@ -47,22 +46,29 @@ go_test(
         "@com_github_ethereum_go_ethereum//event:go_default_library",
         "@com_github_golang_protobuf//proto:go_default_library",
         "@com_github_libp2p_go_floodsub//:go_default_library",
-        "@com_github_libp2p_go_libp2p//p2p/discovery:go_default_library",
         "@com_github_libp2p_go_libp2p//p2p/host/basic:go_default_library",
-        "@com_github_libp2p_go_libp2p_peer//:go_default_library",
         "@com_github_libp2p_go_libp2p_swarm//testing:go_default_library",
         "@com_github_sirupsen_logrus//:go_default_library",
-        "@com_github_sirupsen_logrus//hooks/test:go_default_library",
     ],
 )
 
 # by default gazelle tries to add all the test files to the  first
-# go_test; we want to treat feed_concurrent_test.go differently
-# gazelle:exclude feed_concurrent_test.go
+# go_test; we need to exclude the tests that we want to configure
+# in a particular way
+# gazelle:exclude discovery_norace_test.go
+# gazelle:exclude service_norace_test.go
 
 go_test(
-    name = "go_feed_concurrent_write_test",
-    srcs = ["feed_concurrent_test.go"],
+    name = "go_norace_test",
+    srcs = [
+        "discovery_norace_test.go",
+        "service_norace_test.go",
+    ],
     embed = [":go_default_library"],
-    race = "on",
+    race = "off",  # TODO(#377): fix issues with race detection testing.
+    deps = [
+        "@com_github_libp2p_go_libp2p//p2p/host/basic:go_default_library",
+        "@com_github_libp2p_go_libp2p_swarm//testing:go_default_library",
+        "@com_github_sirupsen_logrus//hooks/test:go_default_library",
+    ],
 )