Skip to content

test(api_remote): rename second-send test to carry _roundtrip_ substring - #137

Merged
TaprootFreak merged 1 commit into
developfrom
fix/second-send-test-prd-skip-naming
May 29, 2026
Merged

test(api_remote): rename second-send test to carry _roundtrip_ substring#137
TaprootFreak merged 1 commit into
developfrom
fix/second-send-test-prd-skip-naming

Conversation

@TaprootFreak

Copy link
Copy Markdown
Contributor

Summary

PR #132 added second_send_succeeds_without_prev_commitment_pubkey_field to the api_remote suite. It's a fully mutating roundtrip test (mints via the publisher, broadcasts commit inscriptions, writes into the SMT/MMR) — structurally identical to send_commit_roundtrip_moves_balance in terms of side effects.

The Deploy PRD workflow excludes mutating roundtrips with --skip _roundtrip_ (substring match, see .github/workflows/deploy-prd.yaml:103-145). The new test's name doesn't contain _roundtrip_, so it slipped through the filter and ran against the live PRD node — failing with 422 Unable to get merkle proofs for provided public key because Mainnet block-times are slow enough that the scanner hadn't yet ingested send-#1's commit inscription into the SMT when send-#2 fired.

DEV e2e passes the same test on the same image (Mutinynet block-times are fast enough that the scanner catches up in-window). The PR #135 protocol fix is verified there. This PR is a pure naming-convention restoration — no behavioural change, no production impact, just putting the test back inside the substring-filter contract that's been in place since the suite was first split into PRD-skip vs DEV-full.

Change

Rename in node/tests/api_remote.rs:1957:

  • second_send_succeeds_without_prev_commitment_pubkey_field
  • second_send_roundtrip_succeeds_without_prev_commitment_pubkey_field

Update the live-server-analogue docstring cross-reference in node/src/account_node_tests.rs:774 to match. The unit test itself (test_send_coins_second_send_succeeds_without_prev_commitment_pubkey) stays — it isn't under the substring-filter contract and the unit-test naming style differs from the e2e style.

Verification

grep -E '^(async )?fn [a-z_]+_roundtrip_' node/tests/api_remote.rs after the rename:

  • mint_roundtrip_lands_balance_and_proof
  • send_commit_roundtrip_moves_balance
  • second_send_roundtrip_succeeds_without_prev_commitment_pubkey_field

Three matches. username_claim_resolve_lnurlp_roundtrip ends with _roundtrip (no trailing underscore) and therefore doesn't match either the regex above OR the workflow's --skip _roundtrip_ filter — it's currently masked by ZKCOINS_E2E_ALLOW_FEATURE_TRIMMED_SERVER because LNURL is off on the PRD MVP image. Not in scope for this PR; flagged for follow-up.

Test plan

  • cargo fmt --check
  • cargo clippy -p node -p shared -- -D warnings
  • cargo clippy -p node --all-features -- -D warnings
  • cargo check --workspace --all-features --tests
  • cargo test -p node --lib account_node::tests::test_send_coins_second_send_succeeds_without_prev_commitment_pubkey -- --test-threads=1 — 1 passed
  • CI green (slim lane — pure rename, no ci:full needed)
  • After deploy: second_send_roundtrip_succeeds_without_prev_commitment_pubkey_field runs on DEV and passes; PRD skips it (no 422 from Mainnet timing race)

Follow-up (out of scope)

  • username_claim_resolve_lnurlp_roundtrip should either be renamed to carry _roundtrip_ as a substring (e.g., username_claim_roundtrip_resolves_lnurlp) or the workflow's filter relaxed to roundtrip without trailing underscore.
  • The deploy-prd.yaml:103-108 comment claiming "three roundtrip tests" needs updating once the substring contract is canonical across all four.

PR #132 added `second_send_succeeds_without_prev_commitment_pubkey_field`
as the fourth mutating roundtrip in `node/tests/api_remote.rs`, but its
name did not contain the `_roundtrip_` substring relied on by
`deploy-prd.yaml`'s `--skip _roundtrip_` filter. The PRD api-e2e job
therefore executed this mutating test against the live PRD node, where
it produced a 422 "Unable to get merkle proofs for provided public key"
because the scanner had not yet ingested the test's send-#1 commit
inscription on a slow Mainnet block-time when send-#2 fired.

Rename to `second_send_roundtrip_succeeds_without_prev_commitment_pubkey_field`
so the test name conforms to the established `<flow>_roundtrip_<assertion>`
pattern used by `mint_roundtrip_lands_balance_and_proof` and
`send_commit_roundtrip_moves_balance`. This restores the substring-filter
contract: every name with `_roundtrip_` mutates publisher UTXOs and the
PRD SMT/MMR, every name without it is read-only.

Also update the live-server-analogue cross-reference in the unit test
docstring at `node/src/account_node_tests.rs:774`. The unit test itself
(`test_send_coins_second_send_succeeds_without_prev_commitment_pubkey`)
is not renamed — it is not under the substring-filter contract.

No behavioural change. The test body, the wallet, and the workflow
`--skip` filter are unchanged.

Signed-off-by: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com>
@TaprootFreak
TaprootFreak marked this pull request as ready for review May 29, 2026 07:38
@TaprootFreak
TaprootFreak merged commit 523026d into develop May 29, 2026
11 checks passed
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.

1 participant