Skip to content

feat(integration): Go-native runner for YAML integration tests (POC)#3537

Open
amir-deris wants to merge 14 commits into
mainfrom
amir/plt-429-go-runner-for-integration-tests-yaml
Open

feat(integration): Go-native runner for YAML integration tests (POC)#3537
amir-deris wants to merge 14 commits into
mainfrom
amir/plt-429-go-runner-for-integration-tests-yaml

Conversation

@amir-deris

@amir-deris amir-deris commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds integration_test/runner/ — a Go-native driver for the existing YAML integration test suite that replaces the Python runner.py for covered modules
  • Keeps all existing YAML test files unchanged; the runner parses the same schema
  • Wires one CI matrix job ("Mint & Staking & Bank Module") to the Go runner as a proof-of-concept, leaving the parallel Autobahn job on Python for side-by-side comparison

Motivation

The Python runner has a few pain points:

  • Failures stop the entire file (exit(1)) with no subtest isolation
  • No -run flag to target a single test case without editing files
  • Output is flat print() statements with no structured test reporting
  • Python dependency in a Go project's CI

The Go runner addresses all of these through standard go test integration.

How it works

Each YAML file maps to a t.Run subtest, so failures are isolated and the full -v output shows every command and its captured output. The eval verifier uses big.Int → float64 → string comparison (in that order) to handle the large token supply integers in the existing YAML assertions without float64 precision loss.

Run a single module locally (requires a running cluster):

go test -tags yaml_integration -v ./integration_test/runner/... -run TestBankModule

Target a single test case:

go test -tags yaml_integration -v ./integration_test/runner/... -run TestBankModule/Test_sending_funds

@cursor

cursor Bot commented Jun 3, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
CI coverage for mint/staking/bank now depends on behavioral parity with runner.py; mismatches could miss regressions without changing production chain code.

Overview
Adds integration_test/runner, a Go driver that runs the existing YAML integration schemas via go test (subtests per case, -run filtering, docker exec into sei-node-0, eval/regex verifiers aligned with the Python runner, including big.Int comparisons for large balances).

The “Mint & Staking & Bank Module” CI matrix job now runs go test -tags yaml_integration ... -run "TestMintModule|TestStakingModule|TestBankModule" instead of three runner.py invocations; the parallel Autobahn job still uses Python for comparison.

gopkg.in/yaml.v3 is a direct module dependency. Small YAML tweaks: multisig test cleans up temp JSON files; simulation dry-run captures stderr (2>&1) for the gas-estimate regex.

Reviewed by Cursor Bugbot for commit 7994319. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJun 10, 2026, 11:03 AM

@codecov

codecov Bot commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 177 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.13%. Comparing base (8027079) to head (7994319).

Files with missing lines Patch % Lines
integration_test/runner/runner.go 0.00% 177 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3537      +/-   ##
==========================================
- Coverage   59.17%   58.13%   -1.05%     
==========================================
  Files        2226     2140      -86     
  Lines      183561   173771    -9790     
==========================================
- Hits       108615   101014    -7601     
+ Misses      65165    63756    -1409     
+ Partials     9781     9001     -780     
Flag Coverage Δ
sei-chain-pr 0.00% <0.00%> (?)
sei-db 70.41% <ø> (ø)
sei-db-state-db ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
integration_test/runner/runner.go 0.00% <0.00%> (ø)

... and 193 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread integration_test/runner/runner.go
Comment thread integration_test/runner/runner_test.go
@amir-deris amir-deris changed the title Added go runner for yaml tests feat(integration): Go-native runner for YAML integration tests (POC) Jun 3, 2026
Comment thread .github/workflows/integration-test.yml Outdated
@amir-deris amir-deris requested review from bdchatham and masih June 3, 2026 12:08
- Run go mod tidy to move gopkg.in/yaml.v3 from indirect to direct deps
- Fix shell quoting in CI YAML: replace single quotes in -run regex with escaped double quotes to avoid breaking the echo '...' wrapper in the test runner script

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread .github/workflows/integration-test.yml Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d74cb62. Configure here.

Comment thread integration_test/runner/runner_test.go Outdated
Comment thread integration_test/runner/runner_test.go Outdated
Comment thread integration_test/runner/runner.go Outdated
Comment thread integration_test/runner/runner.go Outdated
@bdchatham

Copy link
Copy Markdown
Contributor

Not a blocker, and explicitly not a regression in this PR — flagging a pre-existing coverage gap that this work happened to surface, so it's tracked rather than lost.

While reviewing I compared the full suite against what CI actually invokes: 27 YAML files exist, but only 23 are referenced by any CI workflow. Four are orphaned (run by no matrix row):

  • bank_module/multi_sig_send_test.yaml
  • bank_module/simulation_tx.yaml
  • tokenfactory_module/create_tokenfactory_test.yaml
  • template/template_test.yaml ← intentional (it's the authoring example, not a real test)

The first three look accidental, not deliberate:

  • The two bank files were added in autobahn mempool #3522, in the same change that heavily edited integration-test.yml — only send_funds_test.yaml got wired into the matrix; its two siblings didn't.
  • Unlike the genuinely environment-gated tests (the upgrade / statesync cases carry explicit "requires a specially prepared cluster" notes), these carry no skip marker and need no special setup — they're just unreferenced.

This PR faithfully preserves the existing coverage — the Go -run scope reproduces exactly what the Python matrix ran, so nothing is dropped here. If anything the Go runner improves visibility: TestBankModule already wires all three bank files and TestTokenFactoryModule wires the tokenfactory one, so go test ./... makes these orphans discoverable in a way the per-row Python CLI strings never did.

Suggested follow-up (independent of this PR, and applies to both harnesses): wire these three back into CI — in the Python matrix today, and in the Go -run scope as each module migrates — or add an explicit skip marker if any are intentionally held out. Happy to file a tracking issue so it doesn't ride on this POC.

@amir-deris

Copy link
Copy Markdown
Contributor Author

Not a blocker, and explicitly not a regression in this PR — flagging a pre-existing coverage gap that this work happened to surface, so it's tracked rather than lost.

While reviewing I compared the full suite against what CI actually invokes: 27 YAML files exist, but only 23 are referenced by any CI workflow. Four are orphaned (run by no matrix row):

  • bank_module/multi_sig_send_test.yaml
  • bank_module/simulation_tx.yaml
  • tokenfactory_module/create_tokenfactory_test.yaml
  • template/template_test.yaml ← intentional (it's the authoring example, not a real test)

The first three look accidental, not deliberate:

  • The two bank files were added in autobahn mempool #3522, in the same change that heavily edited integration-test.yml — only send_funds_test.yaml got wired into the matrix; its two siblings didn't.
  • Unlike the genuinely environment-gated tests (the upgrade / statesync cases carry explicit "requires a specially prepared cluster" notes), these carry no skip marker and need no special setup — they're just unreferenced.

This PR faithfully preserves the existing coverage — the Go -run scope reproduces exactly what the Python matrix ran, so nothing is dropped here. If anything the Go runner improves visibility: TestBankModule already wires all three bank files and TestTokenFactoryModule wires the tokenfactory one, so go test ./... makes these orphans discoverable in a way the per-row Python CLI strings never did.

Suggested follow-up (independent of this PR, and applies to both harnesses): wire these three back into CI — in the Python matrix today, and in the Go -run scope as each module migrates — or add an explicit skip marker if any are intentionally held out. Happy to file a tracking issue so it doesn't ride on this POC.

@bdchatham Thanks for great feedback, I didn't notice that some of these yaml files are unused actually. It seems these yaml files that are unused were added in this old pr: #949 and not a new thing. I will enable them in the go runner to eliminate this gap.

@masih masih left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice; glad to see python replacement!

This reminds me: why this PR doesn't remove the python runner?

"python3 integration_test/scripts/runner.py integration_test/staking_module/staking_test.yaml",
"python3 integration_test/scripts/runner.py integration_test/bank_module/send_funds_test.yaml",
"python3 integration_test/scripts/runner.py integration_test/mint_module/mint_test.yaml"
"go test -tags yaml_integration -v -timeout 10m ./integration_test/runner/... -run \"TestMintModule|TestStakingModule|TestBankModule\""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please use top level CI timeout (defaults to 30m) or extrapolate via env var.

t.Helper()
data, err := os.ReadFile(path) //nolint:gosec
if err != nil {
t.Fatalf("read %s: %v", path, err)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Use testify? Ditto for the rest of the file.

for i, inp := range tc.Inputs {
container := inp.Node
if container == "" {
container = opts.DefaultContainer

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

non-blocker: I recommend using functional options pattern. That way all defaults are in one place and a lot clearer to reason about gaps in config. Otherwise we end up with zero-value checks across multiple functions which works but easy to miss gaps or manage what the default options should be.

"export PATH=$PATH:"+extraPath+" && "+cmd)
c = exec.Command("docker", args...) //nolint:gosec
} else {
c = exec.Command("/bin/bash", "-c", cmd)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We don't know what the shell is going to be. ideally we should parameterise this, or interpret it from runtime PATH, or stick with sh and make sure commands are compatible with it.

This also completely breaks on non-unix based systems but that's OK for us today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants