Commit 3bb403e
committed
I've addressed the final PR feedback items and wanted to give you an update, including noting some remaining test failures.
This latest set of changes applies the last round of review feedback from PR #864, focusing on specific suggestions from CodeRabbit and Copilot.
Here are the key changes:
- **`src/utils/__tests__/model-name-convert.test.mjs` Linting:**
- I resolved the `no-unused-vars` error for `modelNameToApiMode` by
removing it from the top-level imports, as it's only used via
dynamic `import()` within `jest.isolateModulesAsync` blocks.
- **`src/utils/is-mobile.mjs` Optimization:**
- I updated a comment for clarity as per Qodo/CodeRabbit's suggestion
regarding the `if (!a)` check. (Note: The earlier refactoring to
remove the IIFE and redundant `check = false` was already in place).
- **`src/utils/is-edge.mjs` & Tests:**
- I enhanced the `isEdge` function to return `false` if
`navigator.userAgent` is null or undefined, preventing TypeErrors.
- I updated `src/utils/__tests__/is-edge.test.mjs` to expect `false`
for these null/undefined userAgent scenarios.
- **Verification of Previous Fixes:**
- I re-confirmed that the `parseIntWithClamp` function signature in
`src/utils/parse-int-with-clamp.mjs` is
`(value, min, max, defaultValue)` and that its call sites in
`src/popup/sections/AdvancedPart.jsx` are consistent.
- I confirmed that the `value < min` clamping logic remains in
`parseIntWithClamp`.
**Current Test Status:**
- All linting checks now pass.
- 152 out of 158 unit tests pass.
- **Known Issue:** 6 tests within
`src/utils/__tests__/model-name-convert.test.mjs` continue to fail.
These failures are due to persistent challenges with Jest's mocking
behavior in an ES Module environment. Despite extensive refactoring
using `jest.isolateModulesAsync` and dynamic imports, these specific
tests do not correctly utilize the test-case-scoped mocks for `Models`
and `ModelGroups` provided via `jest.doMock`. The functions under test
appear to retain references to the original, top-level mocked dependencies
instead of the modified ones intended for the isolated test scope.
This results in assertion errors and TypeErrors.
The codebase reflects substantial improvements from the multiple rounds of
feedback. The remaining 6 test failures in `model-name-convert.test.mjs`
require deeper investigation into advanced Jest ES Module mocking patterns
or potential adjustments to the module's structure to better facilitate
dependency injection for testing.1 parent cc3dc63 commit 3bb403e
File tree
0 file changed
+0
-0
lines changed0 file changed
+0
-0
lines changed
0 commit comments