Cover example runner timeout parsing#910
Conversation
✅ Deploy Preview for fedify-json-schema canceled.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRefactors ChangesExample test runner CLI refactor and tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
The example test runner documents --timeout, but the option did not have focused coverage. This extracts argument parsing into a pure helper so the documented forms can be checked without starting examples. While making the module importable, this also aligns the multi-handle example type with the cmd field the runner already uses. Issue: fedify-dev#887 Assisted-by: Codex:gpt-5
Baagheera
force-pushed
the
example-timeout-test
branch
from
0a23695 to
a21d370
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the example test runner by extracting CLI argument parsing into a dedicated parseCliArgs function and adding comprehensive unit tests. It also modularizes the logging configuration and ensures the main execution block only runs when the module is executed directly. Feedback is provided regarding potential NaN propagation when parsing non-numeric values for the --timeout option, which could cause server readiness checks to fail immediately.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/test-examples/mod.ts`:
- Around line 138-141: Validate the timeout value before assigning it in
parseCliArgs: in the `--timeout` and `--timeout=` branches, only consume the
next argument or slice value if it is present and parses to a finite number. If
the value is missing or invalid, leave `defaultTimeoutMs` unchanged and continue
parsing so flags like `--debug` are not skipped; update the `parseCliArgs` logic
around the `arg === "--timeout"` and `arg.startsWith("--timeout=")` checks
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3cb86a7a-83ed-45d7-9c79-522ef626e878
📒 Files selected for processing (4)
CHANGES.mddeno.jsonexamples/test-examples/mod.test.tsexamples/test-examples/mod.ts
Invalid --timeout values could previously become NaN and skip a following flag. Keep the default unless the option receives a positive finite value, and cover the review case. Addresses review feedback on fedify-dev#910. Assisted-by: Codex:gpt-5
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/test-examples/mod.ts`:
- Around line 145-149: The parseCliArgs handling for "--timeout" is leaving
invalid non-flag values like "abc" in the argument stream, which later makes
them get treated as filterNames. Update the "--timeout" branch in parseCliArgs
so the next token is consumed whenever it is present and not another flag, even
if parseTimeoutMs returns null, while still leaving the token untouched only
when it is missing or already starts with "--". Keep the existing
defaultTimeoutMs and i advancement logic aligned with this behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f59e8cfc-bb45-440d-ae8e-c89ea14f4f2a
📒 Files selected for processing (2)
examples/test-examples/mod.test.tsexamples/test-examples/mod.ts
Invalid non-flag values after --timeout should not become example filters. Consume those values while leaving real flags available for normal parsing. Addresses follow-up review feedback on fedify-dev#910. Assisted-by: Codex:gpt-5
The example runner test coverage does not change user-facing API, so leave CHANGES.md untouched. Addresses maintainer feedback on fedify-dev#910. Assisted-by: Codex:gpt-5
2 participants
Summary
--timeout VALUEhandlingimport.meta.mainFixes #887.
Testing
deno fmt --check deno.json examples/test-examples/mod.ts examples/test-examples/mod.test.tsdeno lint examples/test-examples/mod.ts examples/test-examples/mod.test.tsdeno test --check --doc --allow-all --unstable-kv --trace-leaks --parallel examples/test-examples/mod.test.tsdeno run -A examples/test-examples/mod.ts --timeout=123 cloudflare-workersAI assistance
Prepared with assistance from Codex:gpt-5. I reviewed the issue, project instructions, diff, and local test output before opening this PR.