Skip to content

fix(tests/e2e): fix flaky E2E tests with node readiness checks and extended timeout #351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
g0spel wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(tests/e2e): fix flaky E2E tests with node readiness checks and extended timeout #351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 43 additions & 4 deletions tests/e2e/e2e_exec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"context"
"encoding/json"
"fmt"
"net/http"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -726,13 +727,19 @@ func (s *IntegrationTestSuite) expectErrExecValidation(chain *chain, valIdx int,
}

endpoint := fmt.Sprintf("http://%s", s.valResources[chain.id][valIdx].GetHostPort("1317/tcp"))
// Ensure the node is reachable before polling for tx confirmation.
// The REST API may be temporarily unavailable during cold starts or
// chain recovery. We wait up to 2 min for the API to become responsive
// before entering the tx-confirmation poll, which provides a further
// 3 min window. This addresses issue #176.
s.waitForNodeReady(chain, valIdx, endpoint)
// wait for the tx to be committed on chain
s.Require().Eventuallyf(
func() bool {
gotErr := queryKiichainTx(endpoint, txResp.TxHash) != nil
return gotErr == expectErr
},
time.Minute,
3*time.Minute,
5*time.Second,
"stdOut: %s, stdErr: %s",
string(stdOut), string(stdErr),
Expand All @@ -749,14 +756,20 @@ func (s *IntegrationTestSuite) defaultExecValidation(chain *chain, valIdx int) f
}
if strings.Contains(txResp.String(), "code: 0") || txResp.Code == 0 {
endpoint := fmt.Sprintf("http://%s", s.valResources[chain.id][valIdx].GetHostPort("1317/tcp"))
// Wait for the node to be responsive before polling for tx confirmation.
// The REST API can be briefly unavailable during chain cold starts,
// node recovery from stalls, or under resource pressure in CI.
// This pre-check gives the node up to 2 min to become reachable,
// followed by a 3 min window for the tx to be committed (fixes #176).
s.waitForNodeReady(chain, valIdx, endpoint)
s.Require().Eventually(
func() bool {
return queryKiichainTx(endpoint, txResp.TxHash) == nil
},
time.Minute,
3*time.Minute,
5*time.Second,
"stdOut: %s, stdErr: %s",
string(stdOut), string(stdErr),
"tx %s not confirmed after 3 min. endpoint: %s, stdOut: %s, stdErr: %s",
txResp.TxHash, endpoint, string(stdOut), string(stdErr),
)
return true
}
Expand All @@ -779,3 +792,29 @@ func (s *IntegrationTestSuite) execValidationWithError(_ *chain, _ int, errorCon
return false
}
}

// waitForNodeReady ensures the validator node's REST API is responsive before
// proceeding with tx confirmation polling. This mitigates flaky E2E test failures
// (issue #176) caused by:
// - Temporary node unavailability during cold starts / CI cache misses
// - Chain pauses under resource pressure in constrained environments
// - Brief REST API restarts during node recovery
//
// If the node does not become reachable within the timeout, the test fails with
// a diagnostic message that includes the endpoint URL for debugging.
func (s *IntegrationTestSuite) waitForNodeReady(c *chain, valIdx int, endpoint string) {
s.Require().Eventually(
func() bool {
resp, err := http.Get(fmt.Sprintf("%s/cosmos/tx/v1beta1/txs?query=tx.height=0", endpoint))
if err != nil {
return false
}
resp.Body.Close()
return resp.StatusCode == http.StatusOK
},
2*time.Minute,
5*time.Second,
"node %s validator %d REST API (%s) not responding; check container logs for chain status",
c.id, valIdx, endpoint,
)
}
13 changes: 8 additions & 5 deletions tests/e2e/query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,26 @@ import (
func queryKiichainTx(endpoint, txHash string) error {
resp, err := http.Get(fmt.Sprintf("%s/cosmos/tx/v1beta1/txs/%s", endpoint, txHash))
if err != nil {
return fmt.Errorf("failed to execute HTTP request: %w", err)
return fmt.Errorf("failed to query tx %s at %s: %w", txHash, endpoint, err)
}
defer resp.Body.Close()

if resp.StatusCode != 200 {
return fmt.Errorf("tx query returned non-200 status: %d", resp.StatusCode)
return fmt.Errorf("tx %s query returned non-200 status: %d (endpoint: %s)", txHash, resp.StatusCode, endpoint)
}

var result map[string]interface{}

if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
return fmt.Errorf("failed to read response body: %w", err)
return fmt.Errorf("failed to decode tx %s response: %w", txHash, err)
}

txResp := result["tx_response"].(map[string]interface{})
txResp, ok := result["tx_response"].(map[string]interface{})
if !ok {
return fmt.Errorf("tx %s response missing tx_response field", txHash)
}
if v := txResp["code"]; v.(float64) != 0 {
return fmt.Errorf("tx %s failed with status code %v", txHash, v)
return fmt.Errorf("tx %s failed with code %v", txHash, v)
}
Comment on lines +41 to 47

@coderabbitai coderabbitai Bot Jul 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- tests/e2e/query.go ---\n'
nl -ba tests/e2e/query.go | sed -n '1,120p'

printf '\n--- search for TxResponse definition and code tag ---\n'
rg -n 'type TxResponse|json:"code,omitempty"|tx_response|code' -S .

Repository: KiiChain/kiichain

Length of output: 246

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- tests/e2e/query.go ---'
nl -ba tests/e2e/query.go | sed -n '1,120p'

echo
echo '--- TxResponse definition and code tag ---'
rg -n 'type TxResponse|json:"code,omitempty"|json:"code"' -S .

Repository: KiiChain/kiichain

Length of output: 222

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- tests/e2e/query.go ---'
cat -n tests/e2e/query.go | sed -n '1,120p'

echo
echo '--- TxResponse definition ---'
rg -n 'type TxResponse|json:"code,omitempty"|json:"code"' -S .

Repository: KiiChain/kiichain

Length of output: 4759

🌐 Web query:

Cosmos SDK TxResponse json:"code,omitempty" tx_response code omitted on success /cosmos/tx/v1beta1/txs

💡 Result:

In the Cosmos SDK, the code field in the TxResponse object is typically omitted from JSON output on success because it is defined with the omitempty JSON tag [1][2][3]. Since a successful transaction in the Cosmos SDK (specifically at the ABCI level) has a return code of 0 [4], the standard Go json marshaler treats this as a zero value and omits it from the serialized output when the json:"code,omitempty" tag is present. The TxResponse struct (found in cosmos-sdk/types/result.go) is used across various endpoints, including those under /cosmos/tx/v1beta1/txs [1][5][3]. While the underlying protobuf definition and internal Go struct include the code field, its visibility in JSON responses is governed by the serialization rules of the SDK's encoding layer, which adheres to the omitempty convention for this field [1][6]. If you are developing a client (such as a frontend or a service consuming these APIs) and need to verify the success of a transaction, you should not rely solely on the presence or absence of the code field. Instead, standard practice is to: 1. Check if the code field exists and is equal to 0 (if present). 2. Alternatively, treat the absence of the code field as implicitly successful (i.e., code == 0), given that the transaction was successfully processed by the node and returned a TxResponse [4]. For robust client-side logic, it is often recommended to use higher-level libraries (like CosmJS) which handle these serialization nuances and provide helper functions to distinguish between successful and failed transactions [4].

Citations:

Unguarded code assertion can still panic on successful txs. txResp["code"] is omitted on success, so v.(float64) can hit nil and crash this polling path.

🐛 Proposed fix 
-	if v := txResp["code"]; v.(float64) != 0 {
-		return fmt.Errorf("tx %s failed with code %v", txHash, v)
+	code, ok := txResp["code"].(float64)
+	if ok && code != 0 {
+		return fmt.Errorf("tx %s failed with code %v", txHash, code)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
txResp, ok := result["tx_response"].(map[string]interface{})
if !ok {
return fmt.Errorf("tx %s response missing tx_response field", txHash)
}
if v := txResp["code"]; v.(float64) != 0 {
return fmt.Errorf("tx %s failed with status code %v", txHash, v)
return fmt.Errorf("tx %s failed with code %v", txHash, v)
}
txResp, ok := result["tx_response"].(map[string]interface{})
if !ok {
return fmt.Errorf("tx %s response missing tx_response field", txHash)
}
code, ok := txResp["code"].(float64)
if ok && code != 0 {
return fmt.Errorf("tx %s failed with code %v", txHash, code)
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/e2e/query.go` around lines 41 - 47, The tx response handling in the
polling helper still assumes txResp["code"] is always present, which can panic
when it is omitted on successful transactions. Update the logic in the tx
response check to safely read and type-assert the code field in the same area
that inspects tx_response, and only compare it when the field exists; keep the
existing error path for non-zero codes and let missing code be treated as
success.


return nil
Expand Down
Loading