Skip to content

refactor(test-suite): replace fhevm bash cli with lean Bun TS#2134

Merged
mergify[bot] merged 46 commits intomainfrom
elias/fhevm-cli-clean
Apr 2, 2026
Merged

refactor(test-suite): replace fhevm bash cli with lean Bun TS#2134
mergify[bot] merged 46 commits intomainfrom
elias/fhevm-cli-clean

Conversation

@Eikix
Copy link
Copy Markdown
Contributor

@Eikix Eikix commented Mar 19, 2026

Closes https://github.com/zama-ai/fhevm-internal/issues/1042

Supersedes #2066.

#2066 keeps the iteration history and design exploration. This PR is the same final direction with a clean atomic history for review.

Summary

Replace the legacy test-suite/fhevm/fhevm-cli bash flow with a lean Bun/TypeScript CLI.

The new CLI makes target resolution, runtime generation, scenarios, local overrides, resume, upgrade, and test execution explicit instead of spreading that behavior across bash scripts and CI glue.

What changed

  • replace the bash CLI/runtime scripts with a Bun/TS implementation under test-suite/fhevm/src
  • move tracked runtime inputs into clearer buckets:
    • docker-compose/
    • templates/
    • static/
    • profiles/
    • scenarios/
  • keep generated runtime state under .fhevm/ with a hard split between:
    • .fhevm/state
    • .fhevm/runtime
  • support explicit targets:
    • latest-supported
    • latest-main
    • sha --sha <git-sha>
    • devnet
    • testnet
    • mainnet
  • support local overrides and scenario-driven coprocessor topologies
  • keep deploy as an alias for up
  • replace doctor with up --dry-run

CI contract

  • direct PR e2e boots latest-main --build with the checked-in two-of-two scenario
  • PR e2e now runs the bundled ./fhevm-cli test light suite instead of copying those steps in YAML
  • orchestrated merge-queue/release validation calls the reusable e2e workflow with build=false
  • the reusable e2e workflow is explicitly told when it is running in orchestrated mode
  • the workflow now also gates the CLI itself with:
    • bun run check
    • bun test src

Validation

Verified locally on this branch:

  • cd test-suite/fhevm && bun run check
  • cd test-suite/fhevm && bun test src
  • focused live-stack and CI-path validation during the original review cycle

Notes

  • latest-supported is now a tracked maintained profile
  • latest-main is the moving baseline for local and PR validation
  • two-of-two is the canonical checked-in multicopro scenario preset

@Eikix Eikix requested review from a team as code owners March 19, 2026 14:49
@cla-bot cla-bot bot added the cla-signed label Mar 19, 2026
@Eikix Eikix marked this pull request as draft March 19, 2026 14:51
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 19, 2026 •
edited
Loading

Changed Lines Coverage

Coverage of added/modified lines: N/A

Per-file breakdown

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

@Eikix Eikix marked this pull request as ready for review March 20, 2026 10:24
@Eikix Eikix mentioned this pull request Mar 20, 2026
Closed
Eikix
Eikix commented Mar 20, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@Eikix Eikix left a comment

Choose a reason for hiding this comment

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

[AUTOMATED]

Code Review Summary

Reviewed for bugs, guideline compliance, and error handling across all 39 changed files.

Findings

Category Count Severity
Bug 1 High — CI correctness
Naming (guideline) 3 Nit
Error handling 3 Medium

The BUILD env var issue in the e2e workflow is the most impactful finding — it causes unnecessary image rebuilds in the orchestrated CI path. The naming and error handling items are non-blocking improvement suggestions.

Overall the PR is well-structured: the compose path migration is consistent, the CI consolidation is clean, and the documentation is thorough.

Comment thread .github/workflows/test-suite-e2e-tests.yml Outdated
Comment thread test-suite/fhevm/src/generate/env.ts Outdated
Comment thread test-suite/fhevm/src/generate/addresses.ts Outdated
Comment thread test-suite/fhevm/src/resolve/bundle-store.ts Outdated
Comment thread test-suite/fhevm/src/stack.ts Outdated
Comment thread test-suite/fhevm/src/stack.ts Outdated
@Eikix Eikix force-pushed the elias/fhevm-cli-clean branch from 9dd6023 to a2e4ba6 Compare March 24, 2026 09:11
@zama-ai zama-ai deleted a comment from claude bot Mar 25, 2026
Eikix
Eikix commented Mar 25, 2026
View reviewed changes
Comment thread test-suite/fhevm/src/commands/test.ts
Eikix
Eikix commented Mar 25, 2026
View reviewed changes
Comment thread test-suite/fhevm/src/commands/test.ts Outdated
Eikix
Eikix commented Mar 25, 2026
View reviewed changes
Comment thread test-suite/fhevm/src/commands/test.ts Outdated
Eikix
Eikix commented Mar 25, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@Eikix Eikix left a comment

Choose a reason for hiding this comment

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

[AUTOMATED]

Code Review

Reviewed for bugs, security issues, guideline compliance, and error handling.

Result: 1 issue found (error handling). No bugs or security issues detected.

The CLI is well-structured with thorough input validation, proper async/await patterns, correct shell escaping for sh -lc commands, and a solid typed error hierarchy. The 15-step boot pipeline, state persistence, and resume logic all look correct.

Comment thread test-suite/fhevm/src/flow/up-flow.ts Outdated
Eikix
Eikix commented Mar 25, 2026
View reviewed changes
Comment thread .github/workflows/test-suite-e2e-tests.yml
Eikix
Eikix commented Mar 25, 2026
View reviewed changes
Comment thread .github/workflows/test-suite-e2e-tests.yml
@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Mar 25, 2026

Mob review comment: print each version of the stack at start

@Eikix Eikix added the e2e When set on a PR, will enable & trigger a run of e2e tests. label Mar 25, 2026
PanGan21
PanGan21 previously approved these changes Mar 26, 2026
View reviewed changes
@Eikix Eikix added the e2e orchestrate test Run the test-suite-orchestrate-e2e-tests on a PR (instead of merge-queue) label Mar 26, 2026
PanGan21
PanGan21 previously approved these changes Mar 26, 2026
View reviewed changes
@Eikix Eikix force-pushed the elias/fhevm-cli-clean branch 3 times, most recently from 0ef5228 to 20a6625 Compare March 30, 2026 21:14
Eikix
Eikix commented Mar 31, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@Eikix Eikix left a comment

Choose a reason for hiding this comment

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

[AUTOMATED]

Code Review

Reviewed for bugs (2 independent passes), guideline compliance, and error handling.

Bugs: None found. Shell commands use Bun.spawn with argv arrays (no shell injection). SQL uses validated numeric chain IDs and sqlLiteral() escaping. State writes use atomic tmp+rename. Async/await patterns are correct.

Guideline issues: 4 naming nits (abbreviations vs CLAUDE.md naming convention).

Error handling: 3 observability gaps in teardown, cache validation, and container probe paths.

Overall this is solid, well-structured code. No blocking issues.

Comment thread test-suite/fhevm/src/generate/env.ts Outdated
Comment thread test-suite/fhevm/src/cli.ts Outdated
Comment thread test-suite/fhevm/src/flow/up-flow.ts Outdated
Comment thread test-suite/fhevm/src/resolve/bundle-store.ts Outdated
Comment thread test-suite/fhevm/src/flow/up-flow.ts Outdated
tawadaa
tawadaa previously approved these changes Mar 31, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@tawadaa tawadaa left a comment

Choose a reason for hiding this comment

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

LGTM, excellent work, thanks!

Eikix
Eikix commented Apr 1, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@Eikix Eikix left a comment

Choose a reason for hiding this comment

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

[AUTOMATED]

Code Review — PR #2134

Reviewed for bugs, guideline compliance, and error handling.

Summary

  • 1 bug in multi-chain resume logic (phantom container names cause unnecessary re-runs)
  • 1 guideline note about the multi-pass env interpolation engine
  • 4 error handling issues where silent .catch(() => undefined) or bare catch {} swallow diagnostics

Overall this is a strong, well-structured rewrite. The issues below are all fixable without major refactoring.

Comment thread test-suite/fhevm/src/flow/repair.ts Outdated
Comment thread test-suite/fhevm/src/generate/env.ts Outdated
Comment thread test-suite/fhevm/src/utils/process.ts Outdated
Comment thread test-suite/fhevm/src/commands/test.ts Outdated
Comment thread test-suite/fhevm/src/commands/test.ts Outdated
Comment thread test-suite/fhevm/src/flow/runtime-compose.ts Outdated
@Eikix Eikix removed e2e When set on a PR, will enable & trigger a run of e2e tests. e2e orchestrate test Run the test-suite-orchestrate-e2e-tests on a PR (instead of merge-queue) labels Apr 2, 2026
@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Apr 2, 2026

@Mergifyio queue

@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Apr 2, 2026

@Mergifyio refresh

@mergify
Copy link
Copy Markdown

mergify bot commented Apr 2, 2026

queue

☑️ Command queue ignored because it is already running from a previous command.

@mergify
Copy link
Copy Markdown

mergify bot commented Apr 2, 2026

refresh

✅ Pull request refreshed

@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Apr 2, 2026

@Mergifyio dequeue

@Eikix Eikix requested a review from PanGan21 April 2, 2026 18:31
@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Apr 2, 2026

@Mergifyio queue

1 similar comment
@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Apr 2, 2026

@Mergifyio queue

@mergify
Copy link
Copy Markdown

mergify bot commented Apr 2, 2026

queue

☑️ Command queue ignored because it is already running from a previous command.

@mergify
Copy link
Copy Markdown

mergify bot commented Apr 2, 2026 •
edited
Loading

Merge Queue Status

  • 🟠 Waiting for queue conditions
  • ⏳ Enter queue
  • ⏳ Run checks
  • ⏳ Merge
Required conditions to enter a queue
  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of [📌 queue -> configuration change requirements]:
    • -mergify-configuration-changed
    • check-success = Configuration changed
  • any of [🔀 queue conditions]:
    • all of [📌 queue conditions of queue rule main]:
      • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • base = main
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • label!=do-not-merge
      • any of [🛡 GitHub branch protection]:
        • check-success = common-pull-request/lint (bpr)
        • check-neutral = common-pull-request/lint (bpr)
        • check-skipped = common-pull-request/lint (bpr)
      • any of [🛡 GitHub branch protection]:
        • check-success = coprocessor-cargo-test/cargo-tests (bpr)
        • check-neutral = coprocessor-cargo-test/cargo-tests (bpr)
        • check-skipped = coprocessor-cargo-test/cargo-tests (bpr)
      • any of [🛡 GitHub branch protection]:
        • check-success = coprocessor-dependency-analysis/dependencies-check (bpr)
        • check-neutral = coprocessor-dependency-analysis/dependencies-check (bpr)
        • check-skipped = coprocessor-dependency-analysis/dependencies-check (bpr)
      • any of [🛡 GitHub branch protection]:
        • check-skipped = gateway-contracts-deployment-tests/sc-deploy (bpr)
        • check-neutral = gateway-contracts-deployment-tests/sc-deploy (bpr)
        • check-success = gateway-contracts-deployment-tests/sc-deploy (bpr)
      • any of [🛡 GitHub branch protection]:
        • check-skipped = kms-connector-tests/test-connector (bpr)
        • check-neutral = kms-connector-tests/test-connector (bpr)
        • check-success = kms-connector-tests/test-connector (bpr)

@Eikix
Copy link
Copy Markdown
Contributor Author

Eikix commented Apr 2, 2026

@Mergifyio queue

@mergify
Copy link
Copy Markdown

mergify bot commented Apr 2, 2026

queue

☑️ Command queue ignored because it is already running from a previous command.

@eudelins-zama
Copy link
Copy Markdown
Contributor

eudelins-zama commented Apr 2, 2026

@Mergifyio refresh

@mergify
Copy link
Copy Markdown

mergify bot commented Apr 2, 2026

refresh

✅ Pull request refreshed

@eudelins-zama
Copy link
Copy Markdown
Contributor

eudelins-zama commented Apr 2, 2026

@Mergifyio refresh

@mergify
Copy link
Copy Markdown

mergify bot commented Apr 2, 2026

refresh

✅ Pull request refreshed

@mergify
Copy link
Copy Markdown

mergify bot commented Apr 2, 2026 •
edited
Loading

Merge Queue Status

This pull request spent 2 hours 15 minutes 54 seconds in the queue, including 1 hour 14 minutes 48 seconds running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
  • check-success = run-e2e-tests / fhevm-e2e-test
  • any of [🛡 GitHub branch protection]:
    • check-success = common-pull-request/lint (bpr)
    • check-neutral = common-pull-request/lint (bpr)
    • check-skipped = common-pull-request/lint (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-success = coprocessor-cargo-test/cargo-tests (bpr)
    • check-neutral = coprocessor-cargo-test/cargo-tests (bpr)
    • check-skipped = coprocessor-cargo-test/cargo-tests (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-success = coprocessor-dependency-analysis/dependencies-check (bpr)
    • check-neutral = coprocessor-dependency-analysis/dependencies-check (bpr)
    • check-skipped = coprocessor-dependency-analysis/dependencies-check (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-skipped = gateway-contracts-deployment-tests/sc-deploy (bpr)
    • check-neutral = gateway-contracts-deployment-tests/sc-deploy (bpr)
    • check-success = gateway-contracts-deployment-tests/sc-deploy (bpr)
  • any of [🛡 GitHub branch protection]:
    • check-skipped = kms-connector-tests/test-connector (bpr)
    • check-neutral = kms-connector-tests/test-connector (bpr)
    • check-success = kms-connector-tests/test-connector (bpr)

@mergify mergify bot removed the merge-queued label Apr 2, 2026
mergify bot added a commit that referenced this pull request Apr 2, 2026
@mergify
Merge of #2134
beb7942
@mergify mergify bot mentioned this pull request Apr 2, 2026
Closed
51 tasks
@mergify mergify bot merged commit ef080b9 into main Apr 2, 2026
194 of 195 checks passed
@mergify mergify bot deleted the elias/fhevm-cli-clean branch April 2, 2026 21:04
@mergify mergify bot removed the queued label Apr 2, 2026
@Eikix Eikix mentioned this pull request Apr 17, 2026
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tawadaa tawadaa tawadaa approved these changes

@PanGan21 PanGan21 PanGan21 approved these changes

@eudelins-zama eudelins-zama eudelins-zama approved these changes

Assignees

No one assigned

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Eikix @eudelins-zama @tawadaa @PanGan21 @github-advanced-security