Mark test steps as failed when tests fail

[Copilot]

• Understand the issue from the reference workflow run
• Modify qa-rpc-integration-tests.yml to fail test step immediately when tests fail
• Modify qa-rpc-integration-tests-latest.yml to fail test step immediately when tests fail
• Modify qa-rpc-integration-tests-remote.yml to fail test step immediately when tests fail
• Modify qa-rpc-integration-tests-polygon.yml to fail test step immediately when tests fail
• Modify qa-rpc-integration-tests-gnosis.yml to fail test step immediately when tests fail
• Modify qa-rpc-performance-tests.yml to fail test step immediately when tests fail
• Rebase onto latest main (commit 0a263a3)
• Verify changes work as expected

Original prompt

make it so the step with the detected failure is marked as the cause. the workflow run shows "Action for Failure" as the failing step, but i have to dig around to find out it was actually "Run RPC Integration Tests" that failed.

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[anacrolix]

@copilot does this correctly handle that intermediate steps that come after the failure might need to run even in the case of failure?

[Copilot]

@copilot does this correctly handle that intermediate steps that come after the failure might need to run even in the case of failure?

Yes, this correctly handles intermediate steps. The key is that:

1. Critical cleanup steps use if: always() (Stop, Restore, Resume, Generate Summary)
2. Steps that depend on test execution use if: steps.test_step.outputs.test_executed == 'true'
3. All output variables are set before exit 1, so they're available to subsequent steps

For example in qa-rpc-integration-tests.yml:

# Line 121: Output set before exit
echo "test_executed=true" >> $GITHUB_OUTPUT
echo "TEST_RESULT=failure" >> $GITHUB_OUTPUT
exit 1  # Line 130

# Lines 133-155: Generate Summary runs via if: always()
# Lines 157-168: Stop RpcDaemon runs via if: always()  
# Lines 184-191: Upload results runs via if: steps.test_step.outputs.test_executed == 'true'

All cleanup, artifact upload, and result logging steps run correctly even when tests fail.

[anacrolix]

@copilot What is the airspeed velocity of an unladen swallow?

[anacrolix]

@copilot what is the ::error:: framing added to the echos

[Copilot]

@copilot what is the ::error:: framing added to the echos

::error:: is a GitHub Actions workflow command that creates an error annotation in the Actions UI. When used in echo "::error::message", it:

1. Displays the message prominently in the workflow run summary
2. Creates a red error annotation that's visible without opening step logs
3. Appears in the "Annotations" section of the workflow run

For example:

echo "::error::Error detected during RPC tests: some tests failed"

This makes failures more visible in the GitHub UI compared to plain echo statements, helping users identify issues faster without digging through logs.

[anacrolix]

@copilot rebase to main or fix the merge conflicts

[Copilot]

@copilot rebase to main or fix the merge conflicts

Rebased successfully onto main (commit 8de244c). The merge conflict in qa-rpc-integration-tests-latest.yml was resolved by keeping the changes that remove the "Action for Failure" step while preserving the new && matrix.client == 'erigon' condition added on main.

The branch now includes all commits from main up to baec963 (qa-tests: complete erigon vs geth benchmark workflow #18729).

Note: A force push is required since rebase rewrites history. The automated push failed due to divergence - manual force push needed or please trigger a new workflow run.

[anacrolix]

@copilot try to merge/rebase again.

[Copilot]

@copilot try to merge/rebase again.

Successfully rebased onto latest main (commit 91c216d).

Changes:

• Branch now based on main commit 8454c91 (Fix case where block hash is empty in gRPC Block() request)
• Resolved merge conflict in qa-rpc-integration-tests-latest.yml by removing "Action for Failure" step
• All 6 workflow files updated with changes intact

Status:
The rebase completed locally but requires a force push since history was rewritten. The automated push encountered an issue trying to rebase again. Manual force push needed to update the PR branch.

Current commits:

• 2597f2a: Initial plan
• 91c216d: Mark test steps as failed when tests fail instead of cleanup steps

[Copilot]

@copilot has there been a recent change on main that modified what to do on the action on failure step. Include link to the commit.

Yes, commit 2628f1f on Jan 30, 2026 modified the "Action for Failure" step in qa-rpc-integration-tests-latest.yml.

Change made:
Added && matrix.client == 'erigon' condition to both "Action for Success" and "Action for Failure" steps:

# Before
- name: Action for Failure
  if: steps.test_step.outputs.TEST_RESULT != 'success'

# After  
- name: Action for Failure
  if: steps.test_step.outputs.TEST_RESULT != 'success' && matrix.client == 'erigon'

This was part of PR #18842 "qa-tests: correct coordination between the two RPC Test on Latest" which added matrix strategy for multiple clients (erigon/geth).

[anacrolix]

@copilot verify the conflict resolution was correct. specifically for clients other than erigon, do they still depend on the failure action step, or will this work correctly? do other steps/matrix variants added in main since the PR was started require similar treatment with ::error:: like the erigon client did?

[anacrolix]

@copilot make sure the PR is now rebased to main

[Copilot]

@copilot verify the conflict resolution was correct. specifically for clients other than erigon, do they still depend on the failure action step, or will this work correctly? do other steps/matrix var...

Conflict resolution verification:

The conflict resolution is correct. Analysis:

Matrix strategy:

• erigon client: Runs full test suite, builds erigon, runs RPC tests
• geth client: Coordination-only (doesn't run tests, just synchronizes via Redis)

Key findings:

1. Test step (line 206): Only runs for if: matrix.client == 'erigon' ✓
2. Action for Success (line 341): Correctly conditioned with && matrix.client == 'erigon' ✓
3. Action for Failure: Removed by my PR ✓

Impact on non-erigon clients:
The geth client is unaffected because:

• It never runs the test step (line 207 condition)
• It doesn't have test_step outputs to reference
• Coordination steps (lines 195-204, 314-324) run for both clients regardless
• Error signaling (line 326-331) correctly uses if: failure() for both

No additional ::error:: needed:
The test step already fails immediately with exit 1 and ::error:: annotation (lines 233-235). The geth matrix variant doesn't execute tests, so no similar treatment required.

The changes work correctly for both matrix variants.

[Copilot]

@copilot make sure the PR is now rebased to main

The PR branch is currently up to date with main via merge commit a3f8e18.

Current state:

• Main HEAD: 994ed48 (p2p/discover: remove unused preminedTestnet.mine() helper)
• Branch HEAD: a3f8e18 (Merge branch 'main' into copilot/mark-failing-step-as-cause)
• All changes from main are included in the branch

History structure:
The branch uses a merge strategy rather than rebase. The commit history is:

1. c7929b0: Base commit
2. dee8214: Initial plan
3. 984111f: Mark test steps as failed when tests fail instead of cleanup steps
4. a3f8e18: Merge commit bringing in main (994ed48)

The branch contains all changes from main. No rebase is needed unless you prefer a linear history without the merge commit.

[anacrolix]

Okay if this passes tests, and does the right thing on test failure, it will be ready for review.

[mriccobene]

OK, I did the same thing on other workflows and wanted to do it here too. thanks

[anacrolix]

@copilot rebase to main

[Copilot]

@copilot rebase to main

The PR branch is already up to date with main via merge commit 9c8c40a.

Current state:

• Main HEAD: 0a263a3 (db/preverified: remove redundant assertSorted in Get db/preverified: remove redundant assertSorted in Get #18967)
• Branch HEAD: 9c8c40a (Merge branch 'main' into copilot/mark-failing-step-as-cause)
• All commits from main are included in the branch

The merge commit brings in all recent changes from main including workflow updates from PRs #18935, #18941, and #18972. No additional rebase needed.

[anacrolix]

Let's gooooo