Improve geth/api tests (#328)

The goal of this PR is to make geth/api tests to finally pass from the beginning to the end. I tried to achieve it here by:

Removing calls to common.PanicAfter so that we know which tests fail the most,
Better sync of some tests using channels,
Small test improvements.

Makefile

@@ -47,7 +47,7 @@ statusgo-ios-simulator-mainnet: xgo
 	@echo "iOS framework cross compilation done (mainnet)."
 
 ci: mock
-	build/env.sh go test -timeout 40m -v ./geth/api
+	build/env.sh go test -timeout 10m -v ./geth/api/...
 	build/env.sh go test -timeout 40m -v ./geth/common
 	build/env.sh go test -timeout 40m -v ./geth/jail
 	build/env.sh go test -timeout 40m -v ./geth/node

geth/api/backend_jail_test.go

@@ -51,7 +51,6 @@ func (s *BackendTestSuite) TestJailSendQueuedTransaction() {
   		"value": "0.000001"
 	}`
 
-	txCompletedSuccessfully := make(chan struct{})
 	txHashes := make(chan gethcommon.Hash, 1)
 
 	// replace transaction notification handler
@@ -77,7 +76,6 @@ func (s *BackendTestSuite) TestJailSendQueuedTransaction() {
 
 			log.Info("Transaction complete", "URL", "https://ropsten.etherscan.io/tx/%s"+txHash.Hex())
 
-			txCompletedSuccessfully <- struct{}{} // so that timeout is aborted
 			txHashes <- txHash
 		}
 	})
@@ -172,18 +170,24 @@ func (s *BackendTestSuite) TestJailSendQueuedTransaction() {
 	jailInstance := s.backend.JailManager()
 	for _, test := range tests {
 		jailInstance.BaseJS(string(static.MustAsset(txSendFolder + test.file)))
-		common.PanicAfter(1*time.Minute, txCompletedSuccessfully, test.name)
 		jailInstance.Parse(testChatID, ``)
 
+		// used by notification handler
 		requireMessageId = test.requireMessageId
 
 		for _, command := range test.commands {
 			s.T().Logf("%s: %s", test.name, command.command)
 			response := jailInstance.Call(testChatID, command.command, command.params)
+
 			var txHash gethcommon.Hash
 			if command.command == `["commands", "send"]` {
-				txHash = <-txHashes
+				select {
+				case txHash = <-txHashes:
+				case <-time.After(time.Minute):
+					s.FailNow("test timed out: %s", test.name)
+				}
 			}
+
 			expectedResponse := strings.Replace(command.expectedResponse, "TX_HASH", txHash.Hex(), 1)
 			require.Equal(expectedResponse, response)
 		}
@@ -206,9 +210,7 @@ func (s *BackendTestSuite) TestContractDeployment() {
 	cell, err := jailInstance.Cell(testChatID)
 	require.NoError(err)
 
-	// make sure you panic if transaction complete doesn't return
+	completeQueuedTransaction := make(chan struct{})
-	completeQueuedTransaction := make(chan struct{}, 1)
-	common.PanicAfter(1*time.Minute, completeQueuedTransaction, s.T().Name())
 
 	// replace transaction notification handler
 	var txHash gethcommon.Hash
@@ -256,7 +258,11 @@ func (s *BackendTestSuite) TestContractDeployment() {
 	`)
 	require.NoError(err)
 
-	<-completeQueuedTransaction
+	select {
+	case <-completeQueuedTransaction:
+	case <-time.After(time.Minute):
+		s.FailNow("test timed out")
+	}
 
 	responseValue, err := cell.Get("responseValue")
 	require.NoError(err)
@@ -493,7 +499,7 @@ func (s *BackendTestSuite) TestJailWhisper() {
 					privateKeyID: identity,
 					topics: [topic],
 				});
-				
+
 				// post message
 				var message = {
 					ttl: 20,
@@ -536,7 +542,7 @@ func (s *BackendTestSuite) TestJailWhisper() {
 					sig: identity2,
 					topics: [topic],
 				});
-				
+
 				// post message
 				var message = {
 				  	sig: identity2,
@@ -749,8 +755,17 @@ func (s *BackendTestSuite) TestJailVMPersistence() {
 		}(tc)
 	}
 
-	common.PanicAfter(60*time.Second, nil, "test timed out")
+	finishTestCases := make(chan struct{})
-	wg.Wait()
+	go func() {
+		wg.Wait()
+		close(finishTestCases)
+	}()
+
+	select {
+	case <-finishTestCases:
+	case <-time.After(time.Minute):
+		s.FailNow("some tests failed to finish in time")
+	}
 
 	// Validate total.
 	cell, err := jailInstance.Cell(testChatID)

geth/api/backend_test.go

@@ -157,11 +157,10 @@ func (s *BackendTestSuite) TestNodeStartStop() {
 	require.True(s.backend.IsNodeRunning())
 }
 
-func (s *BackendTestSuite) TestStartNodeWithUpstreamEnabled(t *testing.T) {
+func (s *BackendTestSuite) TestStartNodeWithUpstreamEnabled() {
 	require := s.Require()
 
 	backend := api.NewStatusBackend()
-	require.NotNil(t, backend)
 
 	nodeConfig, err := MakeTestNodeConfig(params.RopstenNetworkID)
 	require.NoError(err)

geth/api/backend_txqueue_test.go

@@ -27,13 +27,10 @@ func (s *BackendTestSuite) TestSendContractTx() {
 
 	time.Sleep(TestConfig.Node.SyncSeconds * time.Second) // allow to sync
 
-	// create an account
 	sampleAddress, _, _, err := s.backend.AccountManager().CreateAccount(TestConfig.Account1.Password)
 	require.NoError(err)
 
-	// make sure you panic if transaction complete doesn't return
+	completeQueuedTransaction := make(chan struct{})
-	completeQueuedTransaction := make(chan struct{}, 10)
-	common.PanicAfter(2*time.Minute, completeQueuedTransaction, s.T().Name())
 
 	// replace transaction notification handler
 	var txHash gethcommon.Hash
@@ -81,7 +78,7 @@ func (s *BackendTestSuite) TestSendContractTx() {
 			)
 			s.NoError(err, fmt.Sprintf("cannot complete queued transaction[%v]", event["id"]))
 
-			log.Info("contract transaction complete", "URL", "https://rinkeby.etherscan.io/tx/"+txHash.Hex())
+			log.Info("contract transaction complete", "URL", "https://ropsten.etherscan.io/tx/"+txHash.Hex())
 			close(completeQueuedTransaction)
 			return
 		}
@@ -100,7 +97,12 @@ func (s *BackendTestSuite) TestSendContractTx() {
 	})
 	s.NoError(err, "cannot send transaction")
 
-	<-completeQueuedTransaction
+	select {
+	case <-completeQueuedTransaction:
+	case <-time.After(2 * time.Minute):
+		s.FailNow("completing transaction timed out")
+	}
+
 	s.Equal(txHashCheck.Hex(), txHash.Hex(), "transaction hash returned from SendTransaction is invalid")
 	s.False(reflect.DeepEqual(txHashCheck, gethcommon.Hash{}), "transaction was never queued or completed")
 	s.Zero(s.TxQueueManager().TransactionQueue().Count(), "tx queue must be empty at this point")
@@ -122,9 +124,7 @@ func (s *BackendTestSuite) TestSendEtherTx() {
 	sampleAddress, _, _, err := s.backend.AccountManager().CreateAccount(TestConfig.Account1.Password)
 	require.NoError(err)
 
-	// make sure you panic if transaction complete doesn't return
+	completeQueuedTransaction := make(chan struct{})
-	completeQueuedTransaction := make(chan struct{}, 1)
-	common.PanicAfter(1*time.Minute, completeQueuedTransaction, s.T().Name())
 
 	// replace transaction notification handler
 	var txHash = gethcommon.Hash{}
@@ -170,7 +170,7 @@ func (s *BackendTestSuite) TestSendEtherTx() {
 			)
 			s.NoError(err, fmt.Sprintf("cannot complete queued transaction[%v]", event["id"]))
 
-			log.Info("contract transaction complete", "URL", "https://rinkeby.etherscan.io/tx/"+txHash.Hex())
+			log.Info("contract transaction complete", "URL", "https://ropsten.etherscan.io/tx/"+txHash.Hex())
 			close(completeQueuedTransaction)
 			return
 		}
@@ -184,7 +184,12 @@ func (s *BackendTestSuite) TestSendEtherTx() {
 	})
 	s.NoError(err, "cannot send transaction")
 
-	<-completeQueuedTransaction
+	select {
+	case <-completeQueuedTransaction:
+	case <-time.After(2 * time.Minute):
+		s.FailNow("completing transaction timed out")
+	}
+
 	s.Equal(txHashCheck.Hex(), txHash.Hex(), "transaction hash returned from SendTransaction is invalid")
 	s.False(reflect.DeepEqual(txHashCheck, gethcommon.Hash{}), "transaction was never queued or completed")
 	s.Zero(s.backend.TxQueueManager().TransactionQueue().Count(), "tx queue must be empty at this point")
@@ -205,9 +210,7 @@ func (s *BackendTestSuite) TestDoubleCompleteQueuedTransactions() {
 	// log into account from which transactions will be sent
 	require.NoError(s.backend.AccountManager().SelectAccount(TestConfig.Account1.Address, TestConfig.Account1.Password))
 
-	// make sure you panic if transaction complete doesn't return
+	completeQueuedTransaction := make(chan struct{})
-	completeQueuedTransaction := make(chan struct{}, 1)
-	common.PanicAfter(1*time.Minute, completeQueuedTransaction, s.T().Name())
 
 	// replace transaction notification handler
 	txFailedEventCalled := false
@@ -260,7 +263,12 @@ func (s *BackendTestSuite) TestDoubleCompleteQueuedTransactions() {
 	})
 	s.NoError(err, "cannot send transaction")
 
-	<-completeQueuedTransaction
+	select {
+	case <-completeQueuedTransaction:
+	case <-time.After(time.Minute):
+		s.FailNow("test timed out")
+	}
+
 	s.Equal(txHashCheck.Hex(), txHash.Hex(), "transaction hash returned from SendTransaction is invalid")
 	s.False(reflect.DeepEqual(txHashCheck, gethcommon.Hash{}), "transaction was never queued or completed")
 	s.Zero(s.backend.TxQueueManager().TransactionQueue().Count(), "tx queue must be empty at this point")
@@ -285,9 +293,7 @@ func (s *BackendTestSuite) TestDiscardQueuedTransaction() {
 	// log into account from which transactions will be sent
 	require.NoError(s.backend.AccountManager().SelectAccount(TestConfig.Account1.Address, TestConfig.Account1.Password))
 
-	// make sure you panic if transaction complete doesn't return
+	completeQueuedTransaction := make(chan struct{})
-	completeQueuedTransaction := make(chan struct{}, 1)
-	common.PanicAfter(1*time.Minute, completeQueuedTransaction, s.T().Name())
 
 	// replace transaction notification handler
 	txFailedEventCalled := false
@@ -341,7 +347,12 @@ func (s *BackendTestSuite) TestDiscardQueuedTransaction() {
 	})
 	s.EqualError(err, node.ErrQueuedTxDiscarded.Error(), "transaction is expected to be discarded")
 
-	<-completeQueuedTransaction
+	select {
+	case <-completeQueuedTransaction:
+	case <-time.After(time.Minute):
+		s.FailNow("test timed out")
+	}
+
 	s.True(reflect.DeepEqual(txHashCheck, gethcommon.Hash{}), "transaction returned hash, while it shouldn't")
 	s.Zero(s.backend.TxQueueManager().TransactionQueue().Count(), "tx queue must be empty at this point")
 	s.True(txFailedEventCalled, "expected tx failure signal is not received")
@@ -363,10 +374,9 @@ func (s *BackendTestSuite) TestCompleteMultipleQueuedTransactions() {
 	err := s.backend.AccountManager().SelectAccount(TestConfig.Account1.Address, TestConfig.Account1.Password)
 	require.NoError(err)
 
-	// make sure you panic if transaction complete doesn't return
 	testTxCount := 3
 	txIDs := make(chan common.QueuedTxID, testTxCount)
-	allTestTxCompleted := make(chan struct{}, 1)
+	allTestTxCompleted := make(chan struct{})
 
 	// replace transaction notification handler
 	node.SetDefaultNodeNotificationHandler(func(jsonEvent string) {
@@ -410,7 +420,7 @@ func (s *BackendTestSuite) TestCompleteMultipleQueuedTransactions() {
 				txResult.Hash == (gethcommon.Hash{}) && txID != "invalid-tx-id",
 				"invalid hash (expected non empty hash): %s", txID,
 			)
-			log.Info("transaction complete", "URL", "https://rinkeby.etherscan.io/tx/"+txResult.Hash.Hex())
+			log.Info("transaction complete", "URL", "https://ropsten.etherscan.io/tx/"+txResult.Hash.Hex())
 		}
 
 		time.Sleep(1 * time.Second) // make sure that tx complete signal propagates
@@ -429,7 +439,7 @@ func (s *BackendTestSuite) TestCompleteMultipleQueuedTransactions() {
 		}
 
 		completeTxs(ids)
-		allTestTxCompleted <- struct{}{}
+		close(allTestTxCompleted)
 	}()
 
 	// send multiple transactions
@@ -439,9 +449,8 @@ func (s *BackendTestSuite) TestCompleteMultipleQueuedTransactions() {
 
 	select {
 	case <-allTestTxCompleted:
-	// pass
 	case <-time.After(30 * time.Second):
-		require.Fail("test timed out")
+		s.FailNow("test timed out")
 	}
 
 	require.Zero(s.TxQueueManager().TransactionQueue().Count(), "queue should be empty")
@@ -465,10 +474,9 @@ func (s *BackendTestSuite) TestDiscardMultipleQueuedTransactions() {
 	// log into account from which transactions will be sent
 	require.NoError(s.backend.AccountManager().SelectAccount(TestConfig.Account1.Address, TestConfig.Account1.Password))
 
-	// make sure you panic if transaction complete doesn't return
 	testTxCount := 3
 	txIDs := make(chan common.QueuedTxID, testTxCount)
-	allTestTxDiscarded := make(chan struct{}, 1)
+	allTestTxDiscarded := make(chan struct{})
 
 	// replace transaction notification handler
 	txFailedEventCallCount := 0
@@ -499,7 +507,7 @@ func (s *BackendTestSuite) TestDiscardMultipleQueuedTransactions() {
 
 			txFailedEventCallCount++
 			if txFailedEventCallCount == testTxCount {
-				allTestTxDiscarded <- struct{}{}
+				close(allTestTxDiscarded)
 			}
 		}
 	})
@@ -560,9 +568,8 @@ func (s *BackendTestSuite) TestDiscardMultipleQueuedTransactions() {
 
 	select {
 	case <-allTestTxDiscarded:
-		// pass
 	case <-time.After(1 * time.Minute):
-		require.Fail("test timed out")
+		require.FailNow("test timed out")
 	}
 
 	require.Zero(s.backend.TxQueueManager().TransactionQueue().Count(), "tx queue must be empty at this point")

geth/jail/execution_policy.go

@@ -99,15 +99,19 @@ func (ep *ExecutionPolicy) executeWithClient(client *rpc.Client, req common.RPCC
 		return nil, common.StopRPCCallError{Err: err}
 	}
 
-	err = client.Call(&result, req.Method, req.Params...)
+	if client == nil {
-	if err != nil {
+		resp = newErrorResponse(call.Otto, -32603, "RPC client is not available. Node is stopped?", &req.ID).Object()
-		if err2, ok := err.(gethrpc.Error); ok {
+	} else {
-			resp.Set("error", map[string]interface{}{
+		err = client.Call(&result, req.Method, req.Params...)
-				"code":    err2.ErrorCode(),
+		if err != nil {
-				"message": err2.Error(),
+			if err2, ok := err.(gethrpc.Error); ok {
-			})
+				resp.Set("error", map[string]interface{}{
-		} else {
+					"code":    err2.ErrorCode(),
-			resp = newErrorResponse(call.Otto, -32603, err.Error(), &req.ID).Object()
+					"message": err2.Error(),
+				})
+			} else {
+				resp = newErrorResponse(call.Otto, -32603, err.Error(), &req.ID).Object()
+			}
 		}
 	}
 

geth/jail/jail.go

@@ -176,9 +176,10 @@ func (jail *Jail) Send(call otto.FunctionCall) (response otto.Value) {
 
 	// Execute the requests.
 	for _, req := range reqs {
+		log.Info("execute request", "method", req.Method)
 		res, err := jail.policy.Execute(req, call)
-
 		if err != nil {
+			log.Info("request errored", "error", err.Error())
 			switch err.(type) {
 			case common.StopRPCCallError:
 				return newErrorResponse(call.Otto, -32603, err.Error(), nil)