Skip to content

fix(test): isolate run-loop mocks from run-eval suite#27

Merged
antongulin merged 1 commit into
mainfrom
fix/run-loop-mock-isolation
Jun 17, 2026
Merged

fix(test): isolate run-loop mocks from run-eval suite#27
antongulin merged 1 commit into
mainfrom
fix/run-loop-mock-isolation

Conversation

@antongulin

Copy link
Copy Markdown
Owner

Summary

  • Move mock.module calls in run-loop.test.ts into the test body and restore mocks in finally, so module stubs are not registered at file load time.
  • Run Bun tests with --isolate in test:ts so each test file gets a fresh global scope and mock.module cannot leak across files.

Fixes the flaky publish failure in #28 where run-eval.test.ts failed when run-loop.test.ts ran first:

SyntaxError: Export named 'buildOpenCodeRunCommand' not found in module '.../plugin/lib/run-eval.ts'

Test plan

  • bun test --randomize --isolate test/*.test.ts (10 consecutive runs, 0 failures)
  • bun test test/*.test.ts (42 pass)
  • npm test in plugin/

Made with Cursor

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

🤖 Robin

✅ Finished the review. I found 2 issues.

After you push fixes, comment /review when you are ready for another pass.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 Robin

Review flow: this is a point-in-time review. Push fixes freely, then comment /review when you want Robin to run again.

🔵 1 Low | 💡 1 Suggestions

Summary

The diff improves Bun test isolation by moving module mocks into the test and delaying the run-loop import until after mocks are registered, with cleanup in a finally block. I found no high- or medium-severity runtime, security, or data-loss risks in the diff. The main concerns are maintainability risks around the broad docs/ ignore rule and potential future mock leakage if more tests are added to the same file.


Robin — free AI code reviews for every PR.

Comment thread .gitignore Outdated
Comment thread plugin/test/run-loop.test.ts
Move run-loop module mocks into the test body and run Bun tests with
--isolate so mock.module cannot leak across files and break run-eval
imports during publish prepack.

Co-authored-by: Cursor <cursoragent@cursor.com>
@antongulin antongulin force-pushed the fix/run-loop-mock-isolation branch from 11f16b2 to 8e691ac Compare June 17, 2026 06:55
@antongulin

Copy link
Copy Markdown
Owner Author

/review

Addressed review items in 8e691ac:

  • Dropped unrelated .gitignore change from PR scope via rebase onto main
  • Replied on mock serial suggestion: single test in file; cross-file isolation via bun test --isolate

Verification:

  • bun test --randomize --isolate test/*.test.ts (10 runs, 0 failures)

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

🤖 Robin

✅ Finished the review. I did not find any issues.

After you push fixes, comment /review when you are ready for another pass.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 Robin

Review flow: this is a point-in-time review. Push fixes freely, then comment /review when you want Robin to run again.

No issues found

Summary

The change improves test isolation by moving Bun module mocks inside the test and restoring them in a finally block, which reduces cross-test contamination. The added --isolate flag for TypeScript tests is consistent with that goal. I don't see any blocking correctness, security, reliability, or architecture issues in the diff.


Robin — free AI code reviews for every PR.

@antongulin antongulin merged commit 6584127 into main Jun 17, 2026
1 check passed
@antongulin antongulin deleted the fix/run-loop-mock-isolation branch June 17, 2026 06:58
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