You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Finalize test refactoring and address all PR feedback
This commit concludes a series of refactorings and fixes based on
extensive linting and peer review feedback from PR #864. My primary
focus was on ensuring robust test isolation, particularly in
`src/utils/__tests__/model-name-convert.test.mjs`, and addressing
all actionable suggestions from review bots and you.
Key changes in this final iteration:
- **Robust Test Isolation in `model-name-convert.test.mjs`:**
- I completed refactoring of test cases in `modelNameToDesc`,
`modelNameToApiMode`, `isInApiModeGroup`, and
`isUsingModelName` (partial match scenarios) that required
modified global mock states (`actualMockModels`, `actualMockModelGroups`).
- These tests now consistently and correctly use
`jest.isolateModulesAsync()` with `jest.doMock()` and dynamic
`await import()` to provide test-specific, deep-copied mock states.
This fully prevents mock state leakage and direct modification of
shared mock objects, resolving all related CodeRabbit AI feedback.
- I ensured all `async` test functions using `await` are correctly marked.
- I resolved persistent syntax errors that arose during
previous attempts.
- **Addressed Remaining PR Feedback:**
- `src/utils/is-mobile.mjs`: I enhanced robustness by ensuring the
function returns `false` if all detection methods (userAgent, vendor,
opera) provide no information, instead of throwing an error.
- `src/utils/__tests__/is-mobile.test.mjs`: I updated the corresponding
test case to expect `false` in such scenarios.
- I verified 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` correctly adhere to this
signature, confirming previous fixes were correct and Qodo's
conflicting feedback was likely based on an outdated state.
- I confirmed that other feedback regarding ESLint configuration, radix
for `parseInt`, and comment clarity were previously addressed.
All linting checks (`npm run lint`) pass.
All 157 unit tests in 9 suites pass (`npm test`).
Code coverage for `src/utils/model-name-convert.mjs` is excellent
(Statements: 100%, Branches: 98.63%, Functions: 100%, Lines: 100%),
and other tested utility files also maintain high coverage.
The codebase is now significantly more robust and thoroughly tested.
0 commit comments