Skip to content

test(pyodide): add retry regression coverage for init/package failures#186

Merged
bbopen merged 1 commit intomainfrom
codex/57-pyodide-init-retry-regression-tests
Feb 12, 2026
Merged

test(pyodide): add retry regression coverage for init/package failures#186
bbopen merged 1 commit intomainfrom
codex/57-pyodide-init-retry-regression-tests

Conversation

@bbopen
Copy link
Owner

@bbopen bbopen commented Feb 12, 2026

Summary

  • add regression tests proving PyodideBridge retries init after a loadPyodide failure
  • add regression tests proving retry also works after loadPackage failure
  • keep runtime implementation unchanged; this PR is test coverage only

Validation

  • npx eslint test/runtime_pyodide.test.ts
  • npm test -- test/runtime_pyodide.test.ts

Refs #57

@coderabbitai
Copy link

coderabbitai bot commented Feb 12, 2026

📝 Walkthrough

Walkthrough

Added tests for Pyodide initialization retry behavior, verifying that when loadPyodide or package loading fails once and then succeeds, the system properly retries and resumes normal operation without introducing production code changes.

Changes

Cohort / File(s) Summary
Initialization Retry Tests
test/runtime_pyodide.test.ts
Added two test cases under Initialization Retry: one validating retry behavior when loadPyodide fails with a network error then succeeds, another validating retry behavior when package loading fails then succeeds. Tests verify proper invocation counts and error propagation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 When Pyodide stumbles, we retry with care,
Test upon test, through the digital air,
Networks may fail, packages may stall,
But bounce-back we will—we'll recover from all!
hop hop

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding regression test coverage for Pyodide initialization and package loading retry failures.
Description check ✅ Passed The description is directly related to the changeset, clearly explaining the addition of regression tests for retry behavior after init and package failures.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/57-pyodide-init-retry-regression-tests

No actionable comments were generated in the recent review. 🎉

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2cd5d84 and 44522b1.

📒 Files selected for processing (1)
  • test/runtime_pyodide.test.ts
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2026-01-19T21:00:30.825Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: test/adversarial_playground.test.ts:339-360
Timestamp: 2026-01-19T21:00:30.825Z
Learning: In adversarial tests for the bridge (test/adversarial_playground.test.ts), recovery assertions should be kept in the finally block adjacent to cleanup to verify resilience under stress, even if that means post-timeout checks might mask the original error. The goal is to surface recovery failures as legitimate test failures.

Applied to files:

  • test/runtime_pyodide.test.ts
📚 Learning: 2026-01-20T16:01:14.323Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:599-602
Timestamp: 2026-01-20T16:01:14.323Z
Learning: In `src/runtime/node-bridge.ts` (NodeBridge), a test message is sent to the Python subprocess to confirm the bridge is responsive before marking it as ready.

Applied to files:

  • test/runtime_pyodide.test.ts
📚 Learning: 2026-01-19T21:48:27.823Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 127
File: test/runtime_bridge_fixtures.test.ts:59-66
Timestamp: 2026-01-19T21:48:27.823Z
Learning: In fixture-based tests (e.g., test/runtime_bridge_fixtures.test.ts) and similar tests in the tywrap repository, prefer early returns when Python or fixture files are unavailable. Do not rely on Vitest dynamic skip APIs; a silent pass is intentional for environments lacking Python/fixtures. Treat missing fixtures as optional soft-guards and ensure the test remains non-disruptive in non-availability scenarios.

Applied to files:

  • test/runtime_pyodide.test.ts
📚 Learning: 2026-01-20T01:34:07.064Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 136
File: src/runtime/node.ts:444-458
Timestamp: 2026-01-20T01:34:07.064Z
Learning: When reviewing promise-based polling patterns (e.g., recursive setTimeout in waitForAvailableWorker functions), ensure that any recursive timer is tracked and cleared on timeout or promise resolution to prevent timer leaks. Do not rely on no-op checks after settlement; verify clearTimeout is invoked in all paths and consider using an explicit cancellation (e.g., AbortController) or a guard to cancel pending timers when the promise settles.

Applied to files:

  • test/runtime_pyodide.test.ts
📚 Learning: 2026-01-20T18:37:05.670Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: src/runtime/process-io.ts:37-40
Timestamp: 2026-01-20T18:37:05.670Z
Learning: In the tywrap repo, ESLint is used for linting (not Biome). Do not flag regex literals that contain control characters (e.g., \u001b, \u0000-\u001F) as lint errors, since the current ESLint configuration allows them. When reviewing TypeScript files (e.g., src/**/*.ts), rely on ESLint results and avoid raising issues about these specific control-character regex literals unless there is a competing config change or a policy requiring explicit escaping.

Applied to files:

  • test/runtime_pyodide.test.ts
📚 Learning: 2026-01-20T16:00:49.829Z
Learnt from: bbopen
Repo: bbopen/tywrap PR: 152
File: docs/adr/002-bridge-protocol.md:203-211
Timestamp: 2026-01-20T16:00:49.829Z
Learning: In the BridgeProtocol implementation (tywrap), reject Map and Set explicitly before serialization (e.g., in safeStringify or the serialization entrypoint) with a clear error like "Bridge protocol does not support Map/Set values". Do not rely on post-hoc checks of non-string keys at the point of JSON.stringify, since Maps/Sets cannot round-trip. This should be enforced at the boundary where data is prepared for serialization to ensure deterministic errors and prevent invalid data from propagating.

Applied to files:

  • test/runtime_pyodide.test.ts
🧬 Code graph analysis (1)
test/runtime_pyodide.test.ts (1)
src/runtime/pyodide.ts (1)
  • PyodideBridge (66-88)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: os (windows-latest)
🔇 Additional comments (1)
test/runtime_pyodide.test.ts (1)

502-546: Well-structured retry regression tests — LGTM.

Both tests correctly verify that a failed initialization (either at the loadPyodide or loadPackage stage) resets the internal init state so a subsequent call retries successfully. The mock sequencing with mockRejectedValueOnce / mockResolvedValueOnce is appropriate, and the assertions on call counts and per-call arguments are thorough.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@bbopen bbopen added area:runtime-pyodide Area: Pyodide runtime bridge priority:p2 Priority P2 (medium) labels Feb 12, 2026
@bbopen bbopen merged commit 02db8ff into main Feb 12, 2026
20 checks passed
@bbopen bbopen deleted the codex/57-pyodide-init-retry-regression-tests branch February 12, 2026 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:runtime-pyodide Area: Pyodide runtime bridge priority:p2 Priority P2 (medium)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant