test: add unit tests generated by ToTheos - #217
Conversation
Reviewer's GuideAdds Jest-based unit test infrastructure and AI-generated tests for core translation adapters and types, replacing existing Minimax tests and wiring up ts-jest for TypeScript test execution. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- Consider removing
coverage/coverage-summary.jsonfrom version control and adding thecoveragedirectory to.gitignore, since Jest coverage output is generated artefacts rather than source. - In the new test files you are manually declaring Jest globals (
jest,describe,it,expect) asany; it would be cleaner and safer to rely on@types/jest(or@jest/globals) and proper TS config instead of custom global declarations to preserve type safety.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider removing `coverage/coverage-summary.json` from version control and adding the `coverage` directory to `.gitignore`, since Jest coverage output is generated artefacts rather than source.
- In the new test files you are manually declaring Jest globals (`jest`, `describe`, `it`, `expect`) as `any`; it would be cleaner and safer to rely on `@types/jest` (or `@jest/globals`) and proper TS config instead of custom global declarations to preserve type safety.
## Individual Comments
### Comment 1
<location path="package.json" line_range="25-27" />
<code_context>
"@biomejs/biome": "2.4.15",
"@bob-translate/types": "1.1.0",
"@types/bun": "latest",
+ "@types/jest": "^30.0.0",
+ "jest": "^30.4.2",
+ "ts-jest": "^29.4.10",
"typescript": "5.9.3"
},
</code_context>
<issue_to_address>
**issue (bug_risk):** Jest and ts-jest versions look incompatible and may break the test runner.
`jest` is pinned to `^30.4.2` while `ts-jest` is on `^29.4.10`. Each `ts-jest` major typically targets the same Jest major, and 29.x does not yet officially support Jest 30. This mismatch can cause transform/config failures. Please align the versions (e.g., Jest 29.x with ts-jest 29.x, or upgrade ts-jest once it supports Jest 30) to avoid test runner issues.
</issue_to_address>
### Comment 2
<location path="src/adapter/openai.test.ts" line_range="42-44" />
<code_context>
+const mockStreamRequest = jest.fn();
+
+// Set up global mocks
+Object.defineProperty(global, '$option', {
+ value: mockOption,
+ writable: true,
+});
+
</code_context>
<issue_to_address>
**suggestion (testing):** Global `$option` and `$http` mocks are never restored, which can leak state across test files.
These `Object.defineProperty(global, ...)` calls permanently change `$option`/`$http` for the whole Jest run, so other test suites can be affected depending on order or parallelism. Please snapshot any existing `global.$option`/`global.$http` before overriding and restore them in `afterAll`, or alternatively isolate this via `jest.resetModules()` and local (non-global) mocks so each suite keeps a clean global state.
Suggested implementation:
```typescript
/**
* Snapshot existing global $option before overriding to avoid leaking state across tests.
*/
const originalGlobalOption = (global as any).$option;
const hasOriginalGlobalOption = Object.prototype.hasOwnProperty.call(global, '$option');
// Set up global mocks
Object.defineProperty(global, '$option', {
value: mockOption,
writable: true,
configurable: true,
});
/**
* Restore global $option after all tests in this file complete.
*/
afterAll(() => {
if (hasOriginalGlobalOption) {
Object.defineProperty(global, '$option', {
value: originalGlobalOption,
writable: true,
configurable: true,
});
} else {
delete (global as any).$option;
}
});
```
```typescript
// Jest types for type checking
declare const jest: any;
declare function beforeAll(fn: () => void): void;
declare function afterAll(fn: () => void): void;
declare function beforeEach(fn: () => void): void;
declare function afterEach(fn: () => void): void;
```
1. The same snapshot/restore pattern should be applied to `global.$http`:
- Capture `const originalGlobalHttp = (global as any).$http;` and `const hasOriginalGlobalHttp = Object.prototype.hasOwnProperty.call(global, '$http');` before any `Object.defineProperty(global, '$http', ...)`.
- After overriding with your `$http` mock, add logic in `afterAll` (or a separate `afterAll`) to restore `global.$http` when `hasOriginalGlobalHttp` is `true` and delete it otherwise.
2. Integrate the `$http` restore logic into the same `afterAll` used for `$option` to avoid multiple hooks fighting over the same globals.
3. Ensure any new `$http`-related code you add follows the same `configurable: true` pattern so the properties can be redefined or deleted safely.
</issue_to_address>
### Comment 3
<location path="src/types.test.ts" line_range="15-18" />
<code_context>
+} from './types';
+
+// Add Jest global types
+declare global {
+ const describe: any;
+ const it: any;
+ const expect: any;
+}
+
</code_context>
<issue_to_address>
**suggestion:** Redefining Jest globals as `any` weakens type checking now that `@types/jest` is installed.
These `declare global` stubs override Jest’s real typings and turn `describe`/`it`/`expect` into `any`, negating the benefit of `@types/jest`. Please remove this block and instead rely on Jest’s types (e.g. via `"types": ["jest"]` in tsconfig or a shared `setupTests.d.ts`).
Suggested implementation:
```typescript
TypeCheckConfig,
} from './types';
// Test OpenAiErrorResponse type
describe('OpenAiErrorResponse', () => {
```
To make sure Jest types are available without these `declare global` stubs, confirm one of the following is configured in your project:
1. In `tsconfig.json`, include `"types": ["jest"]` (possibly alongside other type packages) or
2. Have a global declaration file (e.g. `src/setupTests.d.ts` or `types/jest.d.ts`) that contains `/// <reference types="jest" />` or `import '@types/jest';`.
No further changes to `src/types.test.ts` are required once the Jest types are correctly configured.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "@types/jest": "^30.0.0", | ||
| "jest": "^30.4.2", | ||
| "ts-jest": "^29.4.10", |
There was a problem hiding this comment.
issue (bug_risk): Jest and ts-jest versions look incompatible and may break the test runner.
jest is pinned to ^30.4.2 while ts-jest is on ^29.4.10. Each ts-jest major typically targets the same Jest major, and 29.x does not yet officially support Jest 30. This mismatch can cause transform/config failures. Please align the versions (e.g., Jest 29.x with ts-jest 29.x, or upgrade ts-jest once it supports Jest 30) to avoid test runner issues.
| Object.defineProperty(global, '$option', { | ||
| value: mockOption, | ||
| writable: true, |
There was a problem hiding this comment.
suggestion (testing): Global $option and $http mocks are never restored, which can leak state across test files.
These Object.defineProperty(global, ...) calls permanently change $option/$http for the whole Jest run, so other test suites can be affected depending on order or parallelism. Please snapshot any existing global.$option/global.$http before overriding and restore them in afterAll, or alternatively isolate this via jest.resetModules() and local (non-global) mocks so each suite keeps a clean global state.
Suggested implementation:
/**
* Snapshot existing global $option before overriding to avoid leaking state across tests.
*/
const originalGlobalOption = (global as any).$option;
const hasOriginalGlobalOption = Object.prototype.hasOwnProperty.call(global, '$option');
// Set up global mocks
Object.defineProperty(global, '$option', {
value: mockOption,
writable: true,
configurable: true,
});
/**
* Restore global $option after all tests in this file complete.
*/
afterAll(() => {
if (hasOriginalGlobalOption) {
Object.defineProperty(global, '$option', {
value: originalGlobalOption,
writable: true,
configurable: true,
});
} else {
delete (global as any).$option;
}
});// Jest types for type checking
declare const jest: any;
declare function beforeAll(fn: () => void): void;
declare function afterAll(fn: () => void): void;
declare function beforeEach(fn: () => void): void;
declare function afterEach(fn: () => void): void;- The same snapshot/restore pattern should be applied to
global.$http:- Capture
const originalGlobalHttp = (global as any).$http;andconst hasOriginalGlobalHttp = Object.prototype.hasOwnProperty.call(global, '$http');before anyObject.defineProperty(global, '$http', ...). - After overriding with your
$httpmock, add logic inafterAll(or a separateafterAll) to restoreglobal.$httpwhenhasOriginalGlobalHttpistrueand delete it otherwise.
- Capture
- Integrate the
$httprestore logic into the sameafterAllused for$optionto avoid multiple hooks fighting over the same globals. - Ensure any new
$http-related code you add follows the sameconfigurable: truepattern so the properties can be redefined or deleted safely.
| declare global { | ||
| const describe: any; | ||
| const it: any; | ||
| const expect: any; |
There was a problem hiding this comment.
suggestion: Redefining Jest globals as any weakens type checking now that @types/jest is installed.
These declare global stubs override Jest’s real typings and turn describe/it/expect into any, negating the benefit of @types/jest. Please remove this block and instead rely on Jest’s types (e.g. via "types": ["jest"] in tsconfig or a shared setupTests.d.ts).
Suggested implementation:
TypeCheckConfig,
} from './types';
// Test OpenAiErrorResponse type
describe('OpenAiErrorResponse', () => {To make sure Jest types are available without these declare global stubs, confirm one of the following is configured in your project:
- In
tsconfig.json, include"types": ["jest"](possibly alongside other type packages) or - Have a global declaration file (e.g.
src/setupTests.d.tsortypes/jest.d.ts) that contains/// <reference types="jest" />orimport '@types/jest';.
No further changes to src/types.test.ts are required once the Jest types are correctly configured.
AI-Generated Unit Tests
This PR adds AI-generated unit tests to improve coverage and ensure key functionality is tested.
Test coverage summary
Test files and commits
test: add configtest: add src/types.test.tstest: add src/lang.test.tstest: add src/adapter/openai.test.tstest: add src/adapter/index.test.tstest: add src/adapter/base.test.tstest: add src/main.test.tstest: add src/utils/index.test.tsHow to run tests
This contribution was created with assistance from ToTheos (https://totheos.com) to support test generation, code refactoring, and pull request preparation for an open-source codebases.
The tool was used to automate routine software development tasks.
Summary by Sourcery
Add Jest-based unit test suite for core translation adapters and shared types, and configure Jest for TypeScript execution.
Build:
Tests: