Merge pull request #80 from informalsystems/ph/refactor-isConnected

Refactor isConnected and add test for AbciQuery

cometmock/abci_client/client.go

@@ -460,18 +460,6 @@ func (a *AbciClient) SendCheckTx(tx *[]byte) (*abcitypes.ResponseCheckTx, error)
 }
 
 func (a *AbciClient) SendAbciQuery(data []byte, path string, height int64, prove bool) (*abcitypes.ResponseQuery, error) {
-	// find the first connected client
-	var client *AbciCounterpartyClient
-	for _, c := range a.Clients {
-		if c.isConnected {
-			client = &c
-			break
-		}
-	}
-	if client == nil {
-		return nil, fmt.Errorf("no connected clients")
-	}
-
 	// build the Query request
 	request := abcitypes.RequestQuery{
 		Data:   data,
@@ -480,15 +468,30 @@ func (a *AbciClient) SendAbciQuery(data []byte, path string, height int64, prove
 		Prove:  prove,
 	}
 
-	// send Query to the client and collect the response
-	ctx, cancel := context.WithTimeout(context.Background(), ABCI_TIMEOUT)
-	defer cancel()
-	response, err := client.Client.Query(ctx, &request)
-	if err != nil {
-		return nil, err
+	responses := make([]*abcitypes.ResponseQuery, 0)
+
+	for _, client := range a.Clients {
+		// send Query to all clients and collect the responses
+		ctx, cancel := context.WithTimeout(context.Background(), ABCI_TIMEOUT)
+		defer cancel()
+		response, err := client.Client.Query(ctx, &request)
+		if err != nil {
+			return nil, err
+		}
+
+		responses = append(responses, response)
+	}
+
+	if a.ErrorOnUnequalResponses {
+		// return an error if the responses are not all equal
+		for i := 1; i < len(responses); i++ {
+			if !reflect.DeepEqual(responses[i], responses[0]) {
+				return nil, fmt.Errorf("responses are not all equal: %v is not equal to %v", responses[i], responses[0])
+			}
+		}
 	}
 
-	return response, nil
+	return responses[0], nil
 }
 
 // RunEmptyBlocks runs a specified number of empty blocks through ABCI.

cometmock/abci_client/counterparty.go

@@ -14,19 +14,15 @@ type AbciCounterpartyClient struct {
 	Client           abciclient.Client
 	NetworkAddress   string
 	ValidatorAddress string
-	isConnected      bool
 	PrivValidator    types.PrivValidator
 }
 
 // NewAbciCounterpartyClient creates a new AbciCounterpartyClient.
-// It always starts with isConnected = false.
-// Call AbciClient.RetryDisconnectedClients to try to connect to the app.
 func NewAbciCounterpartyClient(client abciclient.Client, networkAddress, validatorAddress string, privValidator types.PrivValidator) *AbciCounterpartyClient {
 	return &AbciCounterpartyClient{
 		Client:           client,
 		NetworkAddress:   networkAddress,
 		ValidatorAddress: validatorAddress,
-		isConnected:      false,
 		PrivValidator:    privValidator,
 	}
 }

cometmock/rpc_server/routes.go

@@ -520,6 +520,9 @@ func ABCIQuery(
 		"ABCIQuery called", "path", "data", "height", "prove", path, data, height, prove)
 
 	response, err := abci_client.GlobalClient.SendAbciQuery(data, path, height, prove)
+	if err != nil {
+		return nil, err
+	}
 
 	abci_client.GlobalClient.Logger.Info(
 		"Response to ABCI query", response.String())

e2e-tests/main_test.go

@@ -53,14 +53,13 @@ func extractHeightFromInfo(jsonBytes []byte) (int, error) {
 	return strconv.Atoi(lastBlockHeight)
 }
 
-// Tests happy path functionality for Abci Info.
-func TestAbciInfo(t *testing.T) {
+func StartChain(t *testing.T) error {
 	// execute the local-testnet-singlechain.sh script
 	t.Log("Running local-testnet-singlechain.sh")
 	cmd := exec.Command("./local-testnet-singlechain.sh", "simd")
 	_, err := runCommandWithOutput(cmd)
 	if err != nil {
-		t.Fatalf("Error running local-testnet-singlechain.sh: %v", err)
+		return fmt.Errorf("Error running local-testnet-singlechain.sh: %v", err)
 	}
 
 	t.Log("Done starting testnet")
@@ -77,11 +76,21 @@ func TestAbciInfo(t *testing.T) {
 		t.Log("Waiting for blocks to be produced, latest output: ", string(out))
 		time.Sleep(1 * time.Second)
 	}
+	return nil
+}
+
+// Tests happy path functionality for Abci Info.
+func TestAbciInfo(t *testing.T) {
+	// start the chain
+	err := StartChain(t)
+	if err != nil {
+		t.Fatalf("Error starting chain: %v", err)
+	}
 
 	// call the abci_info command by calling curl on the REST endpoint
 	// curl -H 'Content-Type: application/json' -H 'Accept:application/json' --data '{"jsonrpc":"2.0","method":"abci_info","id":1}' 127.0.0.1:22331
 	args := []string{"bash", "-c", "curl -H 'Content-Type: application/json' -H 'Accept:application/json' --data '{\"jsonrpc\":\"2.0\",\"method\":\"abci_info\",\"id\":1}' 127.0.0.1:22331"}
-	cmd = exec.Command(args[0], args[1:]...)
+	cmd := exec.Command(args[0], args[1:]...)
 	out, err := runCommandWithOutput(cmd)
 	if err != nil {
 		t.Fatalf("Error running curl\ncommand: %v\noutput: %v\nerror: %v", cmd, string(out), err)
@@ -114,3 +123,33 @@ func TestAbciInfo(t *testing.T) {
 		t.Fatalf("Expected block height to increase, but it did not. First height was %v, second height was %v", height, height2)
 	}
 }
+
+func TestAbciQuery(t *testing.T) {
+	// start the chain
+	err := StartChain(t)
+	if err != nil {
+		t.Fatalf("Error starting chain: %v", err)
+	}
+
+	// call the abci_query command by submitting a query that hits the AbciQuery endpoint
+	// for simplicity, we query for the staking params here - any query would work,
+	// but ones without arguments are easier to construct
+	args := []string{"bash", "-c", "simd q staking params --node tcp://127.0.0.1:22331 --output json"}
+	cmd := exec.Command(args[0], args[1:]...)
+	out, err := runCommandWithOutput(cmd)
+	if err != nil {
+		t.Fatalf("Error running command: %v\noutput: %v\nerror: %v", cmd, string(out), err)
+	}
+
+	// check that the output is valid JSON
+	var data map[string]interface{}
+	if err := json.Unmarshal([]byte(out), &data); err != nil {
+		t.Fatalf("Failed to unmarshal JSON %s \n error was %v", string(out), err)
+	}
+
+	// check that the output contains the expected params field. its contents are not important
+	_, ok := data["params"]
+	if !ok {
+		t.Fatalf("Expected output to contain params field, but it did not. Output was %s", string(out))
+	}
+}