Skip to content

fix: make E2E tests fail-loud in CI instead of silently skipping 🐛#37

Merged
Klazomenai merged 2 commits intomainfrom
fix/5-e2e-fail-loud
Mar 31, 2026
Merged

fix: make E2E tests fail-loud in CI instead of silently skipping 🐛#37
Klazomenai merged 2 commits intomainfrom
fix/5-e2e-fail-loud

Conversation

@Klazomenai
Copy link
Copy Markdown
Owner

@Klazomenai Klazomenai commented Mar 31, 2026

Summary

  • Replace is_node_running with require_node in tests/common/mod.rs
  • In CI (CI env var set): panics if EVM node is unreachable — no more false positives
  • Locally: skips gracefully (existing behaviour preserved)
  • Applied to all 5 E2E test functions that check for a running node
  • Updated module doc comment to reflect Anvil usage and CI behaviour

Refs #5

Test plan

  • Unit + integration tests pass (15 tests)
  • E2E tests skip gracefully without node locally (unset CI)
  • E2E tests panic in CI mode without node (CI=true)
  • CI passes (Rust Tests + Solidity Tests)

Replace is_node_running with require_node that panics when CI env var
is set and no EVM node is reachable. Locally, tests still skip
gracefully. Prevents false positives when Anvil fails to start in CI.

Refs #5
@Klazomenai Klazomenai self-assigned this Mar 31, 2026
@Klazomenai Klazomenai marked this pull request as ready for review March 31, 2026 06:55
Copilot AI review requested due to automatic review settings March 31, 2026 06:55
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Rust E2E test harness to avoid false-positive CI passes by making “missing EVM node” a hard failure under CI while preserving local developer-friendly skipping.

Changes:

  • Replace is_node_running checks in E2E tests with a centralized require_node helper.
  • Make require_node panic when the node is unreachable in CI (CI env var set) and print+skip locally.
  • Update E2E module docs to reflect Anvil/EVM-node usage and CI behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/e2e.rs Switches all node-availability guards to require_node and updates top-level E2E documentation.
tests/common/mod.rs Introduces require_node helper that panics in CI on node unreachability and skips locally.
Comments suppressed due to low confidence (1)

tests/common/mod.rs:117

  • require_node returns false on an invalid/malformed rpc_url parse. In CI this will still cause the calling tests to return early (skip) rather than fail-loud, which reintroduces the false-positive problem this PR is trying to fix. Consider panicking in CI on URL parse errors as well (and optionally include the parse error in the message).
    let url: url::Url = match rpc_url.parse() {
        Ok(u) => u,
        Err(_) => return false,
    };

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Address Copilot review: propagate the underlying error in both the URL
parse and get_block_number failure paths. Also panic in CI on malformed
RPC URL to prevent silent skip false positives.

Refs #5
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Klazomenai Klazomenai merged commit 7c69eec into main Mar 31, 2026
6 checks passed
@Klazomenai Klazomenai deleted the fix/5-e2e-fail-loud branch March 31, 2026 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants