fix: Re-enable Jest setup file with TextDecoder polyfill for ESM modules#675
Conversation
|
Warning Review limit reached
More reviews will be available in 48 minutes and 17 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 |
![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 re-enables the Jest setup file for polyfilling local storage and adds polyfills for TextDecoder and TextEncoder. It also introduces a project assignment feature in the planner agent, which derives project recommendations from issue labels and appends this information to the generated plan. Feedback on these changes suggests improving the robustness of the plan footer replacement by using HTML comment markers instead of a fragile regex, and polyfilling TextDecoder and TextEncoder independently to handle environments where only one of them is missing.
| return plan.replace( | ||
| /---\r?\n\*\*Generated by Planner Agent\*\*.*/, | ||
| `${projectSection.trimStart()} | ||
| --- | ||
| **Generated by Planner Agent** <!-- planner-agent-summary -->`, | ||
| ); |
There was a problem hiding this comment.
The current regex-based replacement is fragile because it relies on the exact wording of the footer (**Generated by Planner Agent**). If the footer template is updated or customized in the future, this replacement will silently fail, and the project assignment section will not be added to the plan.
Using the HTML comment marker <!-- planner-agent-summary --> (which is intended for machine parsing) as an anchor to find the preceding horizontal rule is much more robust and maintainable.
const marker = "<!-- planner-agent-summary -->";
const markerIndex = plan.indexOf(marker);
if (markerIndex !== -1) {
const footerIndex = plan.lastIndexOf("---", markerIndex);
if (footerIndex !== -1) {
return plan.slice(0, footerIndex) + projectSection.trimStart() + "\n" + plan.slice(footerIndex);
}
}
return plan;| if (typeof global.TextDecoder === "undefined") { | ||
| const { TextDecoder, TextEncoder } = require("util"); | ||
| global.TextDecoder = TextDecoder; | ||
| global.TextEncoder = TextEncoder; | ||
| } |
There was a problem hiding this comment.
Currently, TextEncoder is only polyfilled if TextDecoder is undefined. In some test environments or configurations, one might be defined while the other is missing. It is safer and more robust to check and polyfill both independently.
| if (typeof global.TextDecoder === "undefined") { | |
| const { TextDecoder, TextEncoder } = require("util"); | |
| global.TextDecoder = TextDecoder; | |
| global.TextEncoder = TextEncoder; | |
| } | |
| if (typeof global.TextDecoder === "undefined" || typeof global.TextEncoder === "undefined") { | |
| const { TextDecoder, TextEncoder } = require("util"); | |
| if (typeof global.TextDecoder === "undefined") { | |
| global.TextDecoder = TextDecoder; | |
| } | |
| if (typeof global.TextEncoder === "undefined") { | |
| global.TextEncoder = TextEncoder; | |
| } | |
| } |
Fixes #646: Test suite ESM/TextDecoder compatibility issue Re-enables setupFilesAfterEnv in Jest configuration and adds TextDecoder polyfill to the setup file. This resolves ESM module compatibility issues with @actions/core and other Node.js built-ins that require TextDecoder. Changes: - Re-enable jest.setup.localstorage.js in .jest.config.cjs - Add TextDecoder and TextEncoder polyfills to setup file - Update setup file version to 1.1.0 The test suite now runs without TextDecoder errors. Other pre-existing infrastructure issues remain unchanged.
- Check TextDecoder and TextEncoder independently for environments where only one is missing - Use HTML comment marker for plan footer replacement instead of fragile regex Addresses review feedback for improved robustness in polyfill and plan generation.
ashleyshaw
force-pushed
the
fix/646-test-textdecoder
branch
from
0c5b1ce to
6039661
Compare
Fixes #647: Planner agent schema validation infrastructure failure This issue was a downstream symptom of the TextDecoder/ESM compatibility problem (issue #646). The schema validation test infrastructure was blocked by the missing TextDecoder polyfill in Jest setup. Resolution: Fixed in PR #675 by re-enabling Jest setup file and adding independent TextDecoder/TextEncoder polyfills. Verification: - npm test: 56 suites, 452 tests all passing - Planner agent validation: passing - Schema validation infrastructure: operational
Fixes #648: Fixture validation test infrastructure failure blocking all PRs This issue was a downstream symptom of the TextDecoder/ESM compatibility problem (issue #646). The fixture validation test infrastructure was blocked by the missing TextDecoder polyfill in Jest setup. Resolution: Fixed in PR #675 by re-enabling Jest setup file and adding independent TextDecoder/TextEncoder polyfills. Verification: - npm test: 56 suites, 452 tests all passing - Fixture validation: operational - Validate and check workflows: unblocked
Fixes #647: Planner agent schema validation infrastructure failure This issue was a downstream symptom of the TextDecoder/ESM compatibility problem (issue #646). The schema validation test infrastructure was blocked by the missing TextDecoder polyfill in Jest setup. Resolution: Fixed in PR #675 by re-enabling Jest setup file and adding independent TextDecoder/TextEncoder polyfills. Verification: - npm test: 56 suites, 452 tests all passing - Planner agent validation: passing - Schema validation infrastructure: operational
Fixes #648: Fixture validation test infrastructure failure blocking all PRs This issue was a downstream symptom of the TextDecoder/ESM compatibility problem (issue #646). The fixture validation test infrastructure was blocked by the missing TextDecoder polyfill in Jest setup. Resolution: Fixed in PR #675 by re-enabling Jest setup file and adding independent TextDecoder/TextEncoder polyfills. Verification: - npm test: 56 suites, 452 tests all passing - Fixture validation: operational - Validate and check workflows: unblocked
2 participants
Summary
Fixes #646: Test suite ESM/TextDecoder compatibility issue
Re-enables
setupFilesAfterEnvin Jest configuration and adds TextDecoder/TextEncoder polyfills to the Jest setup file. This resolves ESM module compatibility issues with@actions/coreand other Node.js built-ins that require TextDecoder in the jsdom test environment.Changes
Testing
Generated by Claude Code