Skip to content

ci: run integration tests against live Tempo container, remove mocks#123

Open
brendanjryan wants to merge 5 commits intomainfrom
brendan/integration-tests-ci
Open

ci: run integration tests against live Tempo container, remove mocks#123
brendanjryan wants to merge 5 commits intomainfrom
brendan/integration-tests-ci

Conversation

@brendanjryan
Copy link
Copy Markdown
Collaborator

Summary

Run integration tests against a real Tempo devnet + Redis in CI — matching the mppx pattern. Replace mock-based charge tests with integration tests.

CI Changes

  • Add integration job that pulls/caches ghcr.io/tempoxyz/tempo:latest, starts devnet + Redis via docker compose up -d --wait, runs pytest -m integration with TEMPO_RPC_URL and REDIS_URL
  • Added to ci-gate required jobs

Tests Added (integration)

  • test_verify_hash_replay_rejected_with_store
  • test_verify_hash_replay_allowed_without_store
  • test_verify_hash_store_records_on_success
  • test_verify_hash_tx_not_found
  • test_verify_external_id_propagated

Tests Removed (mocks)

  • 5 mock tests from test_tempo.py (now covered by integration)
  • Deleted test_stores_redis.py entirely (all 8 tests covered by test_stores_redis_integration.py)

Tests Kept (mocks correct)

Expiry validation, payload validation, unknown credential type, transport/protocol tests, access key signing, MCP client mocks.

brendanjryan and others added 5 commits April 1, 2026 12:28
When chain_id was omitted from tempo(), the challenge's methodDetails
lacked a chainId field. The Rust CLI (mpp-rs) strictly requires chainId
and rejects challenges without it: 'Malformed payment request: missing
chainId'.

Both mppx and mpp-rs default to 4217 (mainnet). This makes pympp
consistent so servers work out of the box without explicitly passing
chain_id.
The DEFAULT_FEE_PAYER_URL (sponsor.moderato.tempo.xyz) was a testnet-only
service silently used as fallback when no fee payer was configured. With
chain_id now defaulting to mainnet (4217), this would route mainnet
transactions to a testnet sponsor.

Align with mppx and mpp-rs: require explicit fee payer configuration
(either a feePayer account on the method, or a feePayerUrl in
methodDetails). Raise a clear error if neither is set.
Add integration job to CI that pulls the Tempo Docker image, starts
the devnet + Redis via docker compose, and runs integration tests
against the live node — matching the mppx pattern.

Replace mock-based charge tests with integration tests:
- Add 5 new integration tests (hash replay with/without store,
  store recording, tx not found, external ID)
- Remove 5 mock tests from test_tempo.py now covered by integration
- Delete test_stores_redis.py (all tests covered by
  test_stores_redis_integration.py with real Redis)

Keep mock tests for error paths (expiry, payload validation,
unknown credential type) and transport/protocol tests.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21eab7c5ad

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/ci.yml
run: docker compose up -d --wait

- name: Run integration tests
run: uv run pytest -m integration -v --timeout=120
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Run Redis-marked tests in CI integration job

The integration workflow now executes pytest -m integration, but tests/test_stores_redis_integration.py uses the redis marker (not integration), and this commit also deletes tests/test_stores_redis.py. That leaves RedisStore behavior untested in CI (test skips without REDIS_URL, integration excludes non-integration markers), so Redis regressions can merge without detection.

Useful? React with 👍 / 👎.

account: TempoAccount | None = None,
fee_payer: TempoAccount | None = None,
chain_id: int | None = None,
chain_id: int = CHAIN_ID,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep custom rpc_url usable without forcing mainnet chain_id

Defaulting tempo() to chain_id=4217 makes calls like tempo(rpc_url="...", intents=...) implicitly mainnet-bound. That default is propagated into request methodDetails.chainId and client chain-ID validation, so custom/devnet RPC endpoints on non-4217 chains now fail unless callers add an explicit chain_id, which is a regression from the previous None default behavior.

Useful? React with 👍 / 👎.

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