Skip to content

test: expose coding-pattern execution flows - #126

Merged
siyul-park merged 3 commits into
mainfrom
issue-121-125-fixes
Jul 6, 2026
Merged

test: expose coding-pattern execution flows#126
siyul-park merged 3 commits into
mainfrom
issue-121-125-fixes

Conversation

@siyul-park

@siyul-park siyul-park commented Jul 6, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Owner

Summary

  • inline the GVN analysis pass-manager setup so each test shows setup, execution, and assertions directly
  • inline optimizer semantic-preservation execution checks instead of hiding interpreter lifecycle behind a broad helper
  • expand the benchmark run/reset/result-check flow for threaded and JIT benchmark cases

Fixes #121
Fixes #122
Fixes #123

Partially addresses #124 and #125 by focusing this PR on the safe, test-facing coding-pattern violations from the audit. The larger GVN declaration-order and interpreter ownership refactors should remain separate review slices to avoid mixing behavior-neutral source movement with test readability changes.

Testing

Not run locally; repository clone and direct test execution were unavailable in this environment. The changes are limited to test and benchmark structure.

Summary by CodeRabbit

  • Tests
    • Updated optimizer and analysis test coverage to verify results by running programs directly and comparing outputs.
    • Expanded benchmark cases to execute interpreter and JIT paths inline, with clearer per-case timing and result checks.
    • Removed shared test helpers in favor of self-contained test setups.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c9191ec5-50c2-447e-867d-9f1907f725b0

📥 Commits

Reviewing files that changed from the base of the PR and between c84dd91 and 43ae39c.

📒 Files selected for processing (3)
  • analysis/gvn_test.go
  • benchmarks/jit_issue60_test.go
  • optimize/optimizer_test.go
📝 Walkthrough

Walkthrough

Test files across analysis, benchmarks, and optimize packages were refactored to remove shared helper functions (runGVN, runMiniVMProgram, run) and inline the equivalent setup, execution, and assertion logic directly within each test case or benchmark.

Changes

Test helper inlining

Layer / File(s) Summary
Inline GVN pass-manager setup
analysis/gvn_test.go		 Each GVN test case now constructs its own pass.Manager, registers analyses, retrieves *GlobalValueNumbering via pass.GetResult with require.NoError, and asserts on Redundant; the runGVN helper is removed.
Inline JIT benchmark execution
benchmarks/jit_issue60_test.go		 Test cases use single-line struct literals; BenchmarkJITIssue60 inlines interpreter setup, run/assert, reset, and timing for both interp and JIT sub-benchmarks, removing the runMiniVMProgram helper.
Inline optimizer semantic-preservation checks
optimize/optimizer_test.go		 O3 tests for CSE, branch offset repair, control-flow merge redundancy, and type/handler preservation now run before/optimized programs via interp.New(...).Run(...) and compare Pop() results inline instead of using a shared run helper.

Estimated code review effort: 2 (Simple) | ~12 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Test
  participant Optimizer
  participant Interpreter

  Test->>Test: before := build()
  Test->>Optimizer: optimize(build())
  Test->>Interpreter: New().Run(before)
  Interpreter-->>Test: beforeValue = Pop()
  Test->>Interpreter: New().Run(optimized)
  Interpreter-->>Test: optimizedValue = Pop()
  Test->>Test: assert beforeValue == optimizedValue
Loading
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-121-125-fixes

Comment @coderabbitai help to get the list of available commands.

@siyul-park
siyul-park merged commit bb15496 into main Jul 6, 2026
5 of 6 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Jul 10, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

1 participant

@siyul-park