feat: add wrapper-based API testing approach #249
Conversation
* fix: llm script related #234 * refactor: Remove logic to generate files for debug * refactor(llm): clean up script, add error handling, and update docs - Removed unnecessary parts in the LLM script to simplify logic - Added error handling to improve stability and prevent silent failures - Updated related documentation to reflect the revised implementation * chore: add sinon to project dependencies - Installed sinon as a regular dependency - Ensures sinon is available in runtime environment - Edited getItdocPrompt annotation * chore(lockfile): regenerate pnpm-lock.yaml to match package.json
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis change introduces a new wrapper-based testing system with request/response capture and metadata:
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
* fix: apply header key normalized to lowercase * update expected oas.json
json-choi
force-pushed
the
241-feature-add-decorator-based-api-testing-support-for-enhanced
branch
from
df938ff to
da62e90
Compare
Implement high-order function wrapper for Jest/Mocha to capture HTTP requests/responses automatically for API documentation generation. ## Implementation (Draft) ### Core Components - **wrapTest()**: Wraps Jest/Mocha 'it' function with capture context - **request()**: Proxy-based supertest wrapper for request interception - **CaptureContext**: AsyncLocalStorage for thread-safe context management ### Key Features - Automatic HTTP request/response capture during tests - Support for multipart/form-data file uploads (fixes #247) - Thread-safe test isolation using AsyncLocalStorage - Integration with existing TestEventManager/TestResultCollector - Coexists with existing DSL approach (describeAPI/itDoc) ### Test Coverage - Unit tests for CaptureContext, interceptedRequest, wrapTest - Integration tests for complete workflow - Real-world examples: user, product, upload APIs ### Files Added - lib/wrappers/core/CaptureContext.ts - lib/wrappers/core/interceptedRequest.ts - lib/wrappers/wrapTest.ts - lib/wrappers/types.ts - examples/express-ts/src/__tests__/*.wrapper.test.ts - examples/express-ts/src/routes/upload.routes.ts ### Known Issues & TODOs - Needs comprehensive code review - Requires additional edge case testing - Documentation needs improvement - Type definitions may need refinement - ESLint warnings to be addressed - AsyncLocalStorage context timing requires validation This is a draft implementation for issue #241. Further review and testing required before production use. Related: #241, #247
Fix @typescript-eslint/no-unused-expressions errors by adding void keyword to Chai expect assertions and remove unused variable. Changes: - Add 'void' to all standalone expect() assertions in test files - Comment out unused 'apiTest' variable in integration.test.ts - All tests still pass with proper linting Resolves ESLint errors: - CaptureContext.test.ts: 10 expression errors - integration.test.ts: 2 errors (unused var + expression) - interceptedRequest.test.ts: 1 expression error
json-choi
force-pushed
the
241-feature-add-decorator-based-api-testing-support-for-enhanced
branch
from
da62e90 to
172be58
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
examples/express-ts/src/routes/upload.routes.ts (1)
5-5: Consider production-ready storage and size limits.While
memoryStorage()is convenient for development, it loads entire files into memory, which can cause memory issues with large uploads or high concurrency in production.Consider adding size limits and using disk or cloud storage for production:
-const upload = multer({ storage: multer.memoryStorage() }) +const upload = multer({ + storage: multer.memoryStorage(), + limits: { + fileSize: 10 * 1024 * 1024, // 10MB limit + files: 5 + }, + fileFilter: (req, file, cb) => { + // Add validation for allowed file types + const allowedMimes = ['image/jpeg', 'image/png', 'image/gif', 'text/plain', 'application/pdf'] + if (allowedMimes.includes(file.mimetype)) { + cb(null, true) + } else { + cb(new Error('Invalid file type')) + } + } +})examples/express-ts/src/__tests__/upload.wrapper.test.ts (1)
9-10: Use a temporary directory for test file creation.Creating test files in the test source directory (
__dirname) can cause issues with parallel test execution, leftover artifacts if cleanup fails, and potential version control confusion.Apply this diff to use the OS temp directory:
+import os from "os" + const apiTest = wrapTest(it) describe("Upload API - Wrapper Approach", () => { - const testFilePath = path.join(__dirname, "test-file.txt") - const testImagePath = path.join(__dirname, "test-image.png") + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "itdoc-upload-test-")) + const testFilePath = path.join(tempDir, "test-file.txt") + const testImagePath = path.join(tempDir, "test-image.png") beforeAll(() => { // Create test files fs.writeFileSync(testFilePath, "This is a test file content") // Create a simple 1x1 PNG image const pngBuffer = Buffer.from([ 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, // PNG signature 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52, // IHDR chunk 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, // 1x1 dimensions 0x08, 0x02, 0x00, 0x00, 0x00, 0x90, 0x77, 0x53, 0xde, 0x00, 0x00, 0x00, 0x0c, 0x49, 0x44, 0x41, 0x54, 0x08, 0xd7, 0x63, 0xf8, 0xcf, 0xc0, 0x00, 0x00, 0x03, 0x01, 0x01, 0x00, 0x18, 0xdd, 0x8d, 0xb4, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82 // IEND chunk ]) fs.writeFileSync(testImagePath, pngBuffer) })And update the cleanup to remove the entire temp directory:
afterAll(() => { - // Cleanup test files - if (fs.existsSync(testFilePath)) { - fs.unlinkSync(testFilePath) - } - if (fs.existsSync(testImagePath)) { - fs.unlinkSync(testImagePath) - } + // Cleanup temp directory + if (fs.existsSync(tempDir)) { + fs.rmSync(tempDir, { recursive: true, force: true }) + } })Also applies to: 12-29
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
examples/express-ts/package.json(1 hunks)examples/express-ts/src/__tests__/product.wrapper.test.ts(1 hunks)examples/express-ts/src/__tests__/upload.wrapper.test.ts(1 hunks)examples/express-ts/src/__tests__/user.wrapper.test.ts(1 hunks)examples/express-ts/src/index.ts(2 hunks)examples/express-ts/src/routes/upload.routes.ts(1 hunks)lib/dsl/index.ts(1 hunks)lib/wrappers/EXAMPLE.md(1 hunks)lib/wrappers/README.md(1 hunks)lib/wrappers/__tests__/CaptureContext.test.ts(1 hunks)lib/wrappers/__tests__/integration.test.ts(1 hunks)lib/wrappers/__tests__/interceptedRequest.test.ts(1 hunks)lib/wrappers/__tests__/wrapTest.integration.test.ts(1 hunks)lib/wrappers/core/CaptureContext.ts(1 hunks)lib/wrappers/core/interceptedRequest.ts(1 hunks)lib/wrappers/index.ts(1 hunks)lib/wrappers/types.ts(1 hunks)lib/wrappers/wrapTest.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
lib/wrappers/README.md (3)
lib/dsl/interface/describeAPI.ts (1)
method(30-58)examples/express/__tests__/expressApp.test.js (2)
apiDoc(220-245)apiDoc(470-481)examples/nestjs/__test__/app.e2e-spec.ts (1)
apiDoc(12-23)
lib/wrappers/core/interceptedRequest.ts (2)
examples/express-ts/src/index.ts (1)
app(10-10)lib/wrappers/core/CaptureContext.ts (1)
CaptureContext(33-112)
lib/wrappers/types.ts (2)
lib/dsl/index.ts (2)
ApiDocMetadata(24-24)WrappedTestFunction(24-24)lib/wrappers/index.ts (3)
ApiDocMetadata(57-57)TestFunction(57-57)WrappedTestFunction(57-57)
lib/wrappers/wrapTest.ts (3)
lib/wrappers/types.ts (4)
ItFunction(60-60)WrappedTestFunction(65-68)TestFunction(53-53)ApiDocMetadata(20-26)lib/dsl/generator/TestEventManager.ts (1)
TestEventManager(34-106)lib/wrappers/core/CaptureContext.ts (1)
CaptureContext(33-112)
lib/wrappers/core/CaptureContext.ts (2)
lib/wrappers/types.ts (2)
ApiDocMetadata(20-26)CapturedRequest(31-39)lib/wrappers/core/interceptedRequest.ts (1)
request(28-37)
lib/wrappers/__tests__/integration.test.ts (2)
lib/wrappers/wrapTest.ts (1)
wrapTest(48-129)lib/wrappers/core/interceptedRequest.ts (1)
request(28-37)
lib/wrappers/__tests__/interceptedRequest.test.ts (2)
lib/wrappers/core/interceptedRequest.ts (1)
request(28-37)lib/wrappers/core/CaptureContext.ts (1)
CaptureContext(33-112)
lib/wrappers/__tests__/wrapTest.integration.test.ts (2)
lib/wrappers/wrapTest.ts (1)
wrapTest(48-129)lib/wrappers/core/interceptedRequest.ts (1)
request(28-37)
examples/express-ts/src/__tests__/product.wrapper.test.ts (3)
lib/wrappers/wrapTest.ts (1)
wrapTest(48-129)lib/wrappers/core/interceptedRequest.ts (1)
request(28-37)examples/express-ts/src/index.ts (1)
app(10-10)
examples/express-ts/src/__tests__/user.wrapper.test.ts (3)
lib/wrappers/wrapTest.ts (1)
wrapTest(48-129)lib/wrappers/core/interceptedRequest.ts (1)
request(28-37)examples/express-ts/src/index.ts (1)
app(10-10)
lib/wrappers/__tests__/CaptureContext.test.ts (1)
lib/wrappers/core/CaptureContext.ts (1)
CaptureContext(33-112)
examples/express-ts/src/__tests__/upload.wrapper.test.ts (3)
lib/wrappers/wrapTest.ts (1)
wrapTest(48-129)lib/wrappers/core/interceptedRequest.ts (1)
request(28-37)examples/express-ts/src/index.ts (1)
app(10-10)
🪛 GitHub Actions: CI Pipeline
examples/express-ts/src/__tests__/product.wrapper.test.ts
[error] 35-35: GET /api/products/:id should return product with different ID. Expected name to be 'Phone' but received 'Smartphone'.
[error] 140-142: Complete product CRUD workflow: Updated product name did not match expected. Expected 'Workflow Product' but received 'Test Product'.
[error] 168-170: Product filtering and search: Filtering by query params returned 404 instead of 200.
[error] 178-180: Product filtering and search: Searching with multiple params returned 404 instead of 200.
[error] 206-208: Product API with authentication: Custom headers test failed. Expected 200 but got 404.
🪛 markdownlint-cli2 (0.18.1)
lib/wrappers/README.md
199-199: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (6)
examples/express-ts/package.json (1)
23-23: LGTM!The addition of
@types/multerprovides TypeScript type definitions for the multer library used in the new upload routes.examples/express-ts/src/index.ts (1)
6-6: LGTM!The upload routes are properly imported and mounted, following the existing pattern used for user and product routes.
Also applies to: 18-18
lib/wrappers/types.ts (1)
17-68: LGTM!The type definitions are well-structured and appropriately documented. The use of
anyinItFunction(line 60) is intentional for maximum compatibility with Jest and Mocha, as explicitly noted in the comment.lib/dsl/index.ts (1)
22-24: LGTM!The wrapper-based API exports are cleanly integrated into the DSL module, providing a cohesive public API surface for both testing approaches.
lib/wrappers/index.ts (1)
17-57: LGTM!Excellent documentation with clear examples demonstrating both basic usage and metadata support. The module exports are well-organized.
examples/express-ts/src/__tests__/upload.wrapper.test.ts (1)
41-117: LGTM!The test coverage is comprehensive, covering single file uploads, multiple files, uploads with additional fields, error cases, and different file types. The use of
withMetafor documentation metadata is excellent.
json-choi
changed the title
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
lib/wrappers/README.md (1)
198-206: Add language specifier to code block.This is the same issue flagged in the previous review. The fenced code block needs a language identifier for proper rendering and accessibility.
Apply this diff:
-``` +```plaintext 1. wrapTest(it) → 래핑된 테스트 함수 반환 2. apiTest(...) 호출 → AsyncLocalStorage 컨텍스트 생성 3. 사용자 테스트 실행 → createClient.supertest(app) 호출 4. Adapter로 감싸서 반환 → 메서드 호출 캡처 5. .then() 호출 → response 캡처 6. 테스트 성공 → TestResultCollector로 전달 7. 모든 테스트 완료 → OpenAPI 문서 생성Based on static analysis. </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (8)</summary><blockquote> <details> <summary>examples/express-ts/src/__tests__/mocha/upload.wrapper.test.ts (1)</summary><blockquote> `44-120`: **Consider defining response body interfaces.** The repeated use of `(response.body as any)` reduces type safety. Consider defining interfaces for the expected response structures: ```typescript interface SingleFileResponse { message: string file: { originalname: string mimetype: string } } interface MultipleFilesResponse { message: string files: Array<{ originalname: string mimetype: string }> } interface DocumentUploadResponse { message: string document: { title: string description: string file: { originalname: string } } }Then replace
(response.body as any)with typed assertions like(response.body as SingleFileResponse). This provides better IDE support and catches type mismatches at compile time.examples/express-ts/jest.config.ts (1)
12-14: Reconsider suppressing TypeScript error 18046.Suppressing TS error 18046 ("Parameter has a name but no type") may hide legitimate type safety issues in test code. While this error commonly appears in test callbacks (e.g.,
doneparameters), it's better to explicitly type parameters or usevoidwhere appropriate.Consider removing this suppression and explicitly typing test parameters instead:
"ts-jest", { tsconfig: "tsconfig.json", - diagnostics: { - ignoreCodes: [18046], - }, },Then fix any uncovered type issues by adding explicit types to test callbacks.
examples/express-ts/src/__tests__/mocha/product.wrapper.test.ts (1)
76-76: Optional: Consider defining response types for better type safety.Multiple type assertions to
anyreduce TypeScript's type-checking benefits. While acceptable in tests, defining response interfaces would improve maintainability.Example:
interface ProductResponse { id: number name: string price: number category: string } // Then use: expect((response.body as ProductResponse).name).to.equal("Another Product")Also applies to: 107-107, 124-124, 129-129, 138-138
lib/wrappers/adapters/supertest/SupertestAdapter.ts (2)
160-166: Capture error responses in then() for failed requests.Currently errors skip capture. Preserve details when supertest/superagent returns an error with response/res.
public then<T = CapturedResponse>( @@ - return this.test.then( + return this.test.then( (res: any) => { @@ return onFulfilled ? onFulfilled(response) : (response as any) }, (error: any) => { + if (CaptureContext.isActive()) { + const res = (error && (error.response || error.res)) as any + if (res) { + const response: CapturedResponse = { + status: res.status, + statusText: res.statusText || res.statusMessage || `${res.status}`, + headers: res.headers, + body: res.body, + text: res.text, + } + ;(this.capturedData as any).response = response + } + } if (onRejected) return onRejected(error) throw error }, )Also applies to: 178-183
166-172: Use proper statusText; statusType is numeric (1–5), not a message.Prefer Node’s status message if available.
- statusText: res.statusType || `${res.status}`, + statusText: res.res?.statusMessage || `${res.status}`,lib/wrappers/adapters/axios/AxiosAdapter.ts (2)
120-124: Large multipart uploads can fail with Axios defaults; set body limits.When using form-data streams, set maxBodyLength/maxContentLength to Infinity to avoid 413s in Node.
this.config.data = this.formDataObj - - if (!this.config.headers) this.config.headers = {} + if (!this.config.headers) this.config.headers = {} Object.assign(this.config.headers, this.formDataObj.getHeaders()) + ;(this.config as any).maxBodyLength = Infinity + ;(this.config as any).maxContentLength = Infinity
183-185: Document that expect() is a no-op for AxiosAdapter.Either implement basic checks (status/header) or clearly annotate as no-op to avoid false confidence in tests.
lib/wrappers/adapters/fetch/FetchAdapter.ts (1)
78-86: Optional: avoid JSON send overriding multipart.If formData was started (attach/field), a later send() will overwrite body and content-type. Consider guarding send() when formDataObj exists, or routing text fields via field() only.
Also applies to: 110-126, 144-166
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
.serena/.gitignore(1 hunks).serena/project.yml(1 hunks)examples/express-ts/jest.config.ts(1 hunks)examples/express-ts/package.json(2 hunks)examples/express-ts/src/__tests__/jest/product.wrapper.spec.ts(1 hunks)examples/express-ts/src/__tests__/jest/upload.wrapper.spec.ts(1 hunks)examples/express-ts/src/__tests__/jest/user.wrapper.spec.ts(1 hunks)examples/express-ts/src/__tests__/mocha/product.test.ts(1 hunks)examples/express-ts/src/__tests__/mocha/product.wrapper.test.ts(1 hunks)examples/express-ts/src/__tests__/mocha/upload.wrapper.test.ts(1 hunks)examples/express-ts/src/__tests__/mocha/user.test.ts(1 hunks)examples/express-ts/src/__tests__/mocha/user.wrapper.test.ts(1 hunks)examples/express-ts/src/services/productService.ts(3 hunks)lib/dsl/index.ts(1 hunks)lib/wrappers/EXAMPLE.md(1 hunks)lib/wrappers/README.md(1 hunks)lib/wrappers/__tests__/integration.test.ts(1 hunks)lib/wrappers/__tests__/interceptedRequest.test.ts(1 hunks)lib/wrappers/__tests__/wrapTest.integration.test.ts(1 hunks)lib/wrappers/adapters/axios/AxiosAdapter.ts(1 hunks)lib/wrappers/adapters/fetch/FetchAdapter.ts(1 hunks)lib/wrappers/adapters/index.ts(1 hunks)lib/wrappers/adapters/supertest/SupertestAdapter.ts(1 hunks)lib/wrappers/adapters/types.ts(1 hunks)lib/wrappers/index.ts(1 hunks)lib/wrappers/types.ts(1 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (5)
- examples/express-ts/src/tests/mocha/product.test.ts
- .serena/.gitignore
- examples/express-ts/src/tests/mocha/user.test.ts
- lib/wrappers/EXAMPLE.md
- lib/wrappers/tests/interceptedRequest.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- lib/wrappers/types.ts
- lib/wrappers/index.ts
- lib/dsl/index.ts
- lib/wrappers/tests/integration.test.ts
🧰 Additional context used
🧬 Code graph analysis (12)
lib/wrappers/__tests__/wrapTest.integration.test.ts (4)
lib/wrappers/index.ts (2)
wrapTest(53-53)createClient(54-54)lib/wrappers/wrapTest.ts (1)
wrapTest(48-129)lib/wrappers/adapters/index.ts (1)
createClient(43-70)lib/wrappers/adapters/supertest/SupertestAdapter.ts (1)
expect(151-158)
lib/wrappers/adapters/supertest/SupertestAdapter.ts (6)
lib/wrappers/adapters/index.ts (2)
SupertestAdapter(83-83)supertest(49-51)lib/wrappers/adapters/types.ts (5)
HttpAdapter(68-74)HttpClient(55-63)RequestBuilder(24-49)CapturedRequest(18-18)CapturedResponse(18-18)lib/wrappers/types.ts (2)
CapturedRequest(31-47)CapturedResponse(52-58)lib/wrappers/core/CaptureContext.ts (1)
CaptureContext(33-112)lib/wrappers/adapters/axios/AxiosAdapter.ts (1)
field(143-167)lib/wrappers/adapters/fetch/FetchAdapter.ts (1)
field(144-166)
examples/express-ts/src/__tests__/jest/upload.wrapper.spec.ts (3)
lib/wrappers/index.ts (1)
createClient(54-54)lib/wrappers/adapters/index.ts (2)
request(77-79)createClient(43-70)lib/wrappers/adapters/supertest/SupertestAdapter.ts (1)
expect(151-158)
lib/wrappers/adapters/axios/AxiosAdapter.ts (6)
lib/wrappers/adapters/index.ts (3)
AxiosAdapterConfig(85-85)AxiosAdapter(84-84)axios(58-60)lib/wrappers/adapters/types.ts (4)
HttpAdapter(68-74)HttpClient(55-63)RequestBuilder(24-49)CapturedResponse(18-18)lib/wrappers/core/CaptureContext.ts (1)
CaptureContext(33-112)lib/wrappers/adapters/fetch/FetchAdapter.ts (1)
field(144-166)lib/wrappers/adapters/supertest/SupertestAdapter.ts (1)
field(120-135)lib/wrappers/types.ts (1)
CapturedResponse(52-58)
examples/express-ts/src/__tests__/mocha/product.wrapper.test.ts (3)
lib/wrappers/adapters/index.ts (2)
request(77-79)createClient(43-70)examples/express-ts/src/services/productService.ts (1)
ProductService(15-59)lib/wrappers/adapters/supertest/SupertestAdapter.ts (1)
expect(151-158)
examples/express-ts/src/__tests__/jest/user.wrapper.spec.ts (2)
lib/wrappers/adapters/index.ts (2)
request(77-79)createClient(43-70)lib/wrappers/adapters/supertest/SupertestAdapter.ts (1)
expect(151-158)
lib/wrappers/adapters/types.ts (4)
lib/wrappers/adapters/axios/AxiosAdapter.ts (1)
field(143-167)lib/wrappers/adapters/fetch/FetchAdapter.ts (1)
field(144-166)lib/wrappers/adapters/supertest/SupertestAdapter.ts (1)
field(120-135)lib/wrappers/types.ts (1)
CapturedResponse(52-58)
lib/wrappers/adapters/index.ts (4)
lib/wrappers/adapters/types.ts (1)
HttpClient(55-63)lib/wrappers/adapters/supertest/SupertestAdapter.ts (1)
SupertestAdapter(22-26)lib/wrappers/adapters/axios/AxiosAdapter.ts (2)
AxiosAdapterConfig(21-25)AxiosAdapter(27-33)lib/wrappers/adapters/fetch/FetchAdapter.ts (2)
FetchAdapterConfig(20-23)FetchAdapter(25-29)
lib/wrappers/adapters/fetch/FetchAdapter.ts (6)
lib/wrappers/adapters/index.ts (3)
FetchAdapterConfig(87-87)FetchAdapter(86-86)fetch(67-69)lib/wrappers/adapters/types.ts (4)
HttpAdapter(68-74)HttpClient(55-63)RequestBuilder(24-49)CapturedResponse(18-18)lib/wrappers/core/CaptureContext.ts (1)
CaptureContext(33-112)lib/wrappers/adapters/axios/AxiosAdapter.ts (1)
field(143-167)lib/wrappers/adapters/supertest/SupertestAdapter.ts (1)
field(120-135)lib/wrappers/types.ts (1)
CapturedResponse(52-58)
examples/express-ts/src/__tests__/mocha/user.wrapper.test.ts (2)
lib/wrappers/adapters/index.ts (2)
request(77-79)createClient(43-70)lib/wrappers/adapters/supertest/SupertestAdapter.ts (1)
expect(151-158)
examples/express-ts/src/__tests__/jest/product.wrapper.spec.ts (4)
lib/wrappers/index.ts (2)
wrapTest(53-53)createClient(54-54)lib/wrappers/adapters/index.ts (2)
request(77-79)createClient(43-70)examples/express-ts/src/services/productService.ts (1)
ProductService(15-59)lib/wrappers/adapters/supertest/SupertestAdapter.ts (1)
expect(151-158)
examples/express-ts/src/__tests__/mocha/upload.wrapper.test.ts (2)
lib/wrappers/adapters/index.ts (2)
request(77-79)createClient(43-70)lib/wrappers/adapters/supertest/SupertestAdapter.ts (1)
expect(151-158)
🪛 Biome (2.1.2)
lib/wrappers/adapters/supertest/SupertestAdapter.ts
[error] 160-160: Do not add then to a class.
(lint/suspicious/noThenProperty)
lib/wrappers/adapters/axios/AxiosAdapter.ts
[error] 187-187: Do not add then to a class.
(lint/suspicious/noThenProperty)
lib/wrappers/adapters/fetch/FetchAdapter.ts
[error] 185-185: Do not add then to a class.
(lint/suspicious/noThenProperty)
🪛 markdownlint-cli2 (0.18.1)
lib/wrappers/README.md
198-198: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (22)
examples/express-ts/src/__tests__/mocha/upload.wrapper.test.ts (3)
1-16: LGTM! Clean test setup and imports.The imports, wrapTest wrapper, and client creation follow the new wrapper-based testing pattern correctly. The test structure is clear and well-organized.
22-33: Good practice: valid minimal PNG buffer for testing.The hardcoded PNG buffer contains the correct magic bytes (
0x89 0x50 0x4E 0x47) and represents a valid 1×1 pixel PNG, which is an excellent approach for testing without requiring actual image files in the repository.
35-42: Safe cleanup with existence checks.The
afterhook correctly checks for file existence before unlinking, preventing errors if tests fail midway or files are already removed.examples/express-ts/package.json (2)
23-23: LGTM! Appropriate type definitions for multer.The addition of
@types/multeraligns with the PR's objective to support multipart/form-data file uploads. The version ^2.0.0 is appropriate for the multer integration.
11-11: Mocha test glob covers all tests. No orphan.test.tsfiles found outsidesrc/__tests__/mocha; the updated pattern is correct.package.json (3)
130-142: Good practice: Optional peer dependencies for HTTP clients.The use of optional peer dependencies for
axiosandform-datais appropriate. This allows consumers to choose their HTTP client while enabling the wrapper-based testing utilities to support multiple adapters.
102-102: LGTM! Type definitions for lodash.Adding
@types/lodashis appropriate given that lodash is already a runtime dependency (line 92).
108-108: No updates needed for axios and form-data versions
axios ^1.12.2 and form-data ^4.0.4 are the latest stable releases with security patches applied.examples/express-ts/jest.config.ts (1)
6-6: testMatch pattern verification
No.spec.tsfiles were found outside__tests__/jest, so the updatedtestMatchpattern will not omit any existing tests.examples/express-ts/src/services/productService.ts (2)
8-13: LGTM! State management enables test isolation.The shift from a constant array to mutable state with an ID counter properly supports the test reset pattern used in the wrapper tests. This is a clean approach for in-memory test data.
52-58: LGTM! Reset method provides clean test isolation.The
resetProducts()method properly reinitializes both the products array and the ID counter, ensuring tests start with a known state. This aligns well with thebeforeEachhooks in the test files.examples/express-ts/src/__tests__/mocha/product.wrapper.test.ts (1)
17-20: LGTM! Proper test isolation setup.The
beforeEachhook correctly resets the product state before each test, ensuring test independence. This works well with the newresetProducts()method.lib/wrappers/adapters/types.ts (2)
20-49: LGTM! Well-designed builder interface.The
RequestBuilderinterface provides a fluent, chainable API that properly supports all HTTP operations including multipart uploads (attach/field), authentication (auth/bearer), and expectations. The design aligns well with supertest patterns.
51-74: LGTM! Clean adapter interfaces.The
HttpClientandHttpAdapterinterfaces provide a clear abstraction for different HTTP client implementations (supertest, axios, fetch). The generic type parameter onHttpAdapterallows flexible configuration.lib/wrappers/__tests__/wrapTest.integration.test.ts (2)
22-91: LGTM! Comprehensive integration test coverage.The test suite thoroughly validates the wrapper approach with:
- Basic HTTP operations (GET, POST with body, headers, query params)
- Multiple requests in a single test
- Compatibility with chai expect and supertest-style assertions
This provides good confidence in the wrapper functionality.
93-116: LGTM! Metadata handling is well-tested.The metadata tests validate both the
withMetamethod and metadata attachment (summary, tags, description, deprecated), ensuring the API documentation features work correctly.examples/express-ts/src/__tests__/jest/upload.wrapper.spec.ts (2)
14-34: LGTM! Proper test file lifecycle management.The
beforeAllandafterAllhooks correctly create and clean up test files. The PNG buffer is a valid minimal PNG image, suitable for testing file upload functionality.
36-109: LGTM! Comprehensive multipart/form-data test coverage.The tests thoroughly validate multipart upload scenarios per PR objectives (#247):
- Single file uploads with metadata validation
- Multiple file uploads
- Files combined with form fields
- Error handling for missing files
- Different file types (text, images)
This addresses the multipart support requirements from issue #247.
examples/express-ts/src/__tests__/jest/user.wrapper.spec.ts (1)
15-138: LGTM! Comprehensive User API test coverage.The test suite thoroughly validates user operations:
- Registration with success and validation error paths
- Login with valid/invalid credentials
- User retrieval by ID
- Complete end-to-end workflow
The metadata usage demonstrates the wrapper approach effectively.
examples/express-ts/src/__tests__/mocha/user.wrapper.test.ts (1)
16-138: LGTM! Mocha equivalent provides framework compatibility.The test suite mirrors the Jest version with Chai assertions, ensuring the wrapper approach works consistently across both Jest and Mocha frameworks. While this creates some duplication, it's necessary for multi-framework support and validates the framework-agnostic design goal.
examples/express-ts/src/__tests__/jest/product.wrapper.spec.ts (1)
35-41: Assertion alignment fixed.Expectation now matches seed data (“Smartphone”). Suite should pass this case.
lib/wrappers/adapters/index.ts (1)
49-69: Factory and exports look good.createClient and request() alias are clear; docstrings helpful.
Also applies to: 77-79, 81-87
There was a problem hiding this comment.
Actionable comments posted: 23
🧹 Nitpick comments (25)
lib/wrappers/__tests__/interceptedRequest.test.ts (2)
54-54: Remove unnecessaryvoidoperator.The
voidoperator is unnecessary here and could be confusing. The chai assertion will execute correctly without it.Apply this diff:
- void expect(CaptureContext.getCapturedRequests()).to.be.empty + expect(CaptureContext.getCapturedRequests()).to.be.empty
58-223: Consider adding edge-case and error-handling tests.While the current test coverage is solid for happy paths, consider adding tests for:
- Malformed requests (invalid JSON, missing content-type)
- Large request/response bodies (memory implications)
- Binary data handling
- Concurrent request capture (race conditions)
- Capture context cleanup/memory leaks with many requests
- Adapter-level error handling (network failures, timeouts)
These additions would strengthen confidence in the adapter's robustness, especially given this is a core component for automatic HTTP capture.
lib/wrappers/adapters/types.ts (3)
24-49: Replaceanywithunknownfor better type safety.Multiple method signatures use
any, which bypasses TypeScript's type checking:
- Line 25:
send(body: any)- Line 30:
query(params: Record<string, any>)- Line 45:
onRejected?: (error: any) => anyConsider refining these to use
unknownfor parameters, forcing consumers to perform type narrowing, or use generic constraints where appropriate.Example refinements:
- send(body: any): this + send(body: unknown): this - query(params: Record<string, any>): this + query(params: Record<string, string | number | boolean | null>): this then<T = CapturedResponse>( onFulfilled?: (response: CapturedResponse) => T, - onRejected?: (error: any) => any, + onRejected?: (error: unknown) => any, ): Promise<T>
32-32: Consider broader file type support for cross-platform compatibility.The
attach()method currently acceptsstring | Buffer. For better browser and stream support, consider includingBlob,File, orReadableStreamtypes.- attach(field: string, file: string | Buffer, filename?: string): this + attach(field: string, file: string | Buffer | Blob | File | ReadableStream, filename?: string): this
24-49: Add method-level JSDoc for better developer experience.While the interface has high-level documentation, individual methods lack JSDoc comments explaining parameters, return values, and usage examples.
Example:
/** * Set the request body * @param body - Request payload (JSON, form data, etc.) * @returns this builder for chaining */ send(body: unknown): this /** * Set HTTP headers * @param field - Header name * @param value - Header value * @returns this builder for chaining */ set(field: string, value: string): this // ... etcexamples/express-ts/src/__tests__/jest/product.wrapper.spec.ts (4)
35-41: Add metadata for consistency.This test lacks metadata while the previous test includes it. For documentation completeness and consistency, consider adding metadata via
withMeta.- apiTest("should return product with different ID", async () => { + apiTest.withMeta({ + summary: "Get product by ID (alternative)", + tags: ["Products"], + })("should return product with different ID", async () => {
56-60: Hardcoded ID assertion may be fragile.Line 57 asserts the new product will have
id: 3, assuming the reset leaves products with IDs 1 and 2. If the reset logic or initial seed data changes, this assertion will break.Consider either:
- Remove the hardcoded ID assertion and only verify the other properties
- Verify
idexists and is a number without asserting the specific value- Add a comment explaining the assumption
expect(response.status).toBe(201) - expect(response.body).toHaveProperty("id", 3) + expect(response.body).toHaveProperty("id") + expect(typeof response.body.id).toBe("number")
97-106: Add metadata for consistency.This test lacks metadata while other tests include it. Consider adding metadata for documentation completeness.
- apiTest("should update product with partial data", async () => { + apiTest.withMeta({ + summary: "Update product (partial)", + tags: ["Products", "Update"], + })("should update product with partial data", async () => {
195-211: Verify deletion actually occurred.These tests only check the
204status code but don't verify the product was actually deleted from the system. Consider adding a follow-up GET request to confirm the product no longer exists.apiTest.withMeta({ summary: "Delete product", tags: ["Products", "Delete"], description: "Deletes a product by its ID", })("should delete a product", async () => { const response = await request.delete("/api/products/1") expect(response.status).toBe(204) + + // Verify product is actually deleted + const getResponse = await request.get("/api/products/1") + expect(getResponse.status).toBe(404) })Also, consider adding metadata to the second test for consistency.
examples/express-ts/src/services/productService.ts (1)
52-58: Consider documenting the testing-only purpose.The
resetProductsmethod properly reinstates the initial state for test isolation. However, since it's exposed in the public service API, consider adding a JSDoc comment clarifying this is intended for testing purposes to prevent misuse if this example code is used as a template.Example:
+ /** + * Resets products to initial state. + * @internal This method is intended for testing purposes only. + */ resetProducts: () => { products = [ { id: 1, name: "Laptop", price: 999.99, category: "Electronics" }, { id: 2, name: "Smartphone", price: 699.99, category: "Electronics" }, ] nextId = 3 },lib/wrappers/index.ts (1)
17-51: Enhance documentation examples for clarity.The JSDoc examples would benefit from showing complete, runnable code:
- Line 24: The import path
'itdoc/wrappers'should be verified against the actual package structure (might need to be'itdoc'or include a subpath export configuration).- Line 26: The example assumes
itis available but doesn't show the import (import { it } from '@jest/globals'or similar).- Lines 30, 44: The
appvariable is used without showing its setup (e.g.,import app from './app').Consider this more complete example structure:
* @example Basic usage * ```typescript - * import { wrapTest, createClient } from 'itdoc/wrappers' + * import { describe, it, expect } from '@jest/globals' + * import { wrapTest, createClient } from 'itdoc' + * import app from './app' * * const apiTest = wrapTest(it)examples/express-ts/src/__tests__/mocha/upload.wrapper.test.ts (2)
25-31: Consider adding a comment explaining the PNG structure.The hardcoded PNG buffer serves its purpose for testing, but a brief comment explaining that this is a minimal 1x1 PNG would improve readability.
Apply this diff to add clarifying context:
+ // Minimal 1x1 PNG image for testing (89 50 4E 47 = PNG signature) const pngBuffer = Buffer.from([ 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48,
35-42: Consider adding error handling for file deletion failures.The existence checks before deletion are good practice, but silent failures during
unlinkSynccould mask cleanup issues.Apply this diff to add basic error handling:
after(() => { - if (fs.existsSync(testFilePath)) { - fs.unlinkSync(testFilePath) - } - if (fs.existsSync(testImagePath)) { - fs.unlinkSync(testImagePath) - } + try { + if (fs.existsSync(testFilePath)) { + fs.unlinkSync(testFilePath) + } + if (fs.existsSync(testImagePath)) { + fs.unlinkSync(testImagePath) + } + } catch (error) { + console.warn("Failed to clean up test files:", error) + } })lib/wrappers/__tests__/wrapTest.integration.test.ts (2)
42-91: Consider adding negative test cases and edge cases.The "basic usage" suite only covers happy paths. For comprehensive integration testing, consider adding tests for:
- Invalid request bodies (malformed JSON)
- Missing required fields
- Special characters in URLs/parameters
- Empty request bodies
- Large payloads
- Request timeout scenarios
Example additions:
apiTest("should handle malformed JSON gracefully", async () => { // Test how the system handles invalid JSON }) apiTest("should handle special characters in query params", async () => { const response = await createClient .supertest(app) .get("/users") .query({ search: "[email protected]" }) expect(response.status).to.equal(200) }) apiTest("should handle empty request body", async () => { const response = await createClient .supertest(app) .post("/users") .send({}) // Assert appropriate response })
93-116: Verify that metadata is captured and propagated.While the tests set metadata using
.withMeta(), they don't verify that this metadata is actually captured and available for documentation generation. This is important to ensure the feature works end-to-end.Add assertions that verify metadata propagation:
apiTest.withMeta({ summary: "Create User", tags: ["Users", "Registration"], })("should create user with metadata", async () => { const response = await createClient.supertest(app).post("/users").send({ name: "Jane", email: "[email protected]", }) expect(response.status).to.equal(201) // TODO: Verify that metadata was captured // const capturedData = getCapturedDataFromContext() // expect(capturedData.metadata.summary).to.equal("Create User") // expect(capturedData.metadata.tags).to.deep.equal(["Users", "Registration"]) })examples/express-ts/src/__tests__/mocha/product.wrapper.test.ts (3)
22-44: Consider adding edge-case tests for error scenarios.While the happy-path tests are correct, consider adding tests for:
- Non-existent product IDs (404 responses)
- Invalid ID formats (e.g., non-numeric)
- Error handling
65-77: Type assertions reduce type safety.Line 76 uses
(response.body as any).namewhich bypasses TypeScript's type checking. While common in tests, consider defining a proper response type or using type guards for better type safety.Example refactor:
interface ProductResponse { id: number; name: string; price: number; category: string; } // Then in test: expect((response.body as ProductResponse).name).to.equal("Another Product")
111-144: Validate extracted product ID before use.Line 124 extracts the product ID using
(createResponse.body as any).idwithout validating the response structure. If the create endpoint fails to return an ID, subsequent requests will fail cryptically.Consider validating the response before extracting the ID:
expect(createResponse.status).to.equal(201) +expect(createResponse.body).to.have.property("id") const productId = (createResponse.body as any).idexamples/express-ts/src/__tests__/mocha/user.wrapper.test.ts (1)
30-30: Consider adding proper type definitions.The type assertion
(response.body as any)bypasses type safety. If response body types are available from the itdoc wrapper system or can be defined locally, using them would improve type safety.Example:
-expect((response.body as any).user).to.have.property("username", "testuser") +interface RegisterResponse { + message: string; + user: { username: string }; +} +expect((response.body as RegisterResponse).user).to.have.property("username", "testuser")examples/express-ts/src/__tests__/jest/upload.wrapper.spec.ts (2)
10-34: Consider usingos.tmpdir()for test file creation.Creating test files in
__dirnameworks but is less portable. Using Node'sos.tmpdir()would ensure tests run reliably in containerized or restricted environments.Example refactor:
+import os from 'os' + describe("Upload API - Wrapper Approach", () => { - const testFilePath = path.join(__dirname, "test-file.txt") - const testImagePath = path.join(__dirname, "test-image.png") + const testFilePath = path.join(os.tmpdir(), "test-file.txt") + const testImagePath = path.join(os.tmpdir(), "test-image.png")
36-109: Test coverage is solid for core multipart/form-data scenarios.The test suite effectively demonstrates the new wrapper-based approach and covers the main multipart upload scenarios mentioned in issue #247:
- Single file upload ✓
- Multiple file upload ✓
- File upload with text fields ✓
- Error handling (missing file) ✓
- Binary file support (PNG) ✓
All assertions are appropriate and the metadata usage aligns well with documentation generation goals.
Optional enhancement: Consider adding edge case tests for:
- File size limits (if enforced)
- Invalid file types or extensions (if restricted)
- Malformed multipart requests
- Empty files (zero bytes)
These would provide more comprehensive coverage but are not blockers for the current implementation.
lib/wrappers/adapters/index.ts (2)
49-51: Consider more specific typing for the app parameter.The
app: anytype provides maximum flexibility for different frameworks (Express, Fastify, NestJS), but could be refined if there's a common interface these frameworks share. For instance:type SupportedApp = Express.Application | FastifyInstance | /* other types */;However, the current
anytype is acceptable given the multi-framework support requirements.
72-79: Enhance the deprecation notice with version and timeline information.The deprecation notice could be more actionable by including:
- The version in which it was deprecated
- The version when it will be removed
- Migration timeline or guidance
Example:
/** * Backward compatibility: keep `request` as alias to `createClient.supertest` * @param app * @deprecated since v2.0.0. Use createClient.supertest() instead. * This alias will be removed in v3.0.0. */lib/wrappers/adapters/fetch/FetchAdapter.ts (2)
78-86: Add error handling for JSON serialization.
JSON.stringify()can throw on circular references, BigInt values, or other non-serializable content. Consider adding a try-catch to provide a clearer error message.public send(body: any): this { - this.init.body = JSON.stringify(body) + try { + this.init.body = JSON.stringify(body) + } catch (error) { + throw new Error(`Failed to serialize request body: ${error}`) + } this.headers["Content-Type"] = "application/json"
185-224: Consider checking Content-Type before parsing JSON.The code currently attempts to parse every response as JSON (lines 201-204), regardless of the
Content-Typeheader. While the fallback to text is reasonable, this could be inefficient for large non-JSON responses.Consider checking Content-Type first:
const res = await fetch(fullURL, this.init) + const contentType = res.headers.get('content-type') || '' const text = await res.text() let body: any - try { - body = JSON.parse(text) - } catch { - body = text + + if (contentType.includes('application/json')) { + try { + body = JSON.parse(text) + } catch { + body = text + } + } else { + body = text }Alternatively, if you want to preserve the current "try JSON first" behavior but skip the attempt for obvious non-JSON types:
const res = await fetch(fullURL, this.init) + const contentType = res.headers.get('content-type') || '' const text = await res.text() let body: any - try { - body = JSON.parse(text) - } catch { - body = text + + if (!contentType.includes('text/html') && !contentType.includes('text/plain')) { + try { + body = JSON.parse(text) + } catch { + body = text + } + } else { + body = text }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
.serena/.gitignore(1 hunks).serena/project.yml(1 hunks)examples/express-ts/jest.config.ts(1 hunks)examples/express-ts/package.json(2 hunks)examples/express-ts/src/__tests__/jest/product.wrapper.spec.ts(1 hunks)examples/express-ts/src/__tests__/jest/upload.wrapper.spec.ts(1 hunks)examples/express-ts/src/__tests__/jest/user.wrapper.spec.ts(1 hunks)examples/express-ts/src/__tests__/mocha/product.test.ts(1 hunks)examples/express-ts/src/__tests__/mocha/product.wrapper.test.ts(1 hunks)examples/express-ts/src/__tests__/mocha/upload.wrapper.test.ts(1 hunks)examples/express-ts/src/__tests__/mocha/user.test.ts(1 hunks)examples/express-ts/src/__tests__/mocha/user.wrapper.test.ts(1 hunks)examples/express-ts/src/services/productService.ts(3 hunks)lib/dsl/index.ts(1 hunks)lib/wrappers/EXAMPLE.md(1 hunks)lib/wrappers/README.md(1 hunks)lib/wrappers/__tests__/integration.test.ts(1 hunks)lib/wrappers/__tests__/interceptedRequest.test.ts(1 hunks)lib/wrappers/__tests__/wrapTest.integration.test.ts(1 hunks)lib/wrappers/adapters/axios/AxiosAdapter.ts(1 hunks)lib/wrappers/adapters/fetch/FetchAdapter.ts(1 hunks)lib/wrappers/adapters/index.ts(1 hunks)lib/wrappers/adapters/supertest/SupertestAdapter.ts(1 hunks)lib/wrappers/adapters/types.ts(1 hunks)lib/wrappers/index.ts(1 hunks)lib/wrappers/types.ts(1 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- .serena/.gitignore
- lib/wrappers/README.md
- examples/express-ts/src/tests/mocha/user.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/express-ts/package.json
- lib/wrappers/types.ts
🔇 Additional comments (29)
examples/express-ts/jest.config.ts (1)
6-6: Verify that the narrowed testMatch pattern doesn't exclude existing tests.The pattern now only matches tests in
__tests__/jest/**/*.spec.ts, which is more restrictive than typical Jest patterns. Ensure that no existing test files outside this pattern are unintentionally excluded.Run the following script to check for any test files that may no longer be matched:
lib/wrappers/__tests__/interceptedRequest.test.ts (1)
43-256: Add tests for multipart/form-data file uploads.The PR objectives explicitly mention supporting multipart/form-data for file uploads (issue #247), which is a key feature of this change. However, this test suite doesn't include any tests for multipart requests or file uploads. Add comprehensive tests covering:
- File upload scenarios with text fields
- Binary data handling
- Multiple file uploads
- Mixed multipart content (files + JSON fields)
Run the following script to check if multipart tests exist elsewhere:
Based on the PR objectives and issue #247.
lib/wrappers/adapters/supertest/SupertestAdapter.ts (1)
168-168: Verify statusType fallback semantics.Line 168 uses
res.statusTypeif available, otherwise stringifiesres.status. EnsurestatusTypeis a documented Supertest property and that the fallback correctly represents the HTTP status text (e.g., "200" vs. "OK").Run the following script to check Supertest's response structure:
lib/wrappers/adapters/types.ts (3)
17-18: LGTM: Clean re-export pattern.The re-export of
CapturedResponseandCapturedRequestprovides a convenient single import point for adapter consumers while maintaining proper type source.
55-63: LGTM: Clean HTTP client interface.The
HttpClientinterface follows REST conventions, provides all standard HTTP verbs, and returnsRequestBuilderfor fluent method chaining. The design is straightforward and effective.
68-74: LGTM: Verify consistent implementation across adapters.The
HttpAdapterinterface uses a clean factory pattern with generic configuration. This design allows for flexible adapter implementations (Supertest, Axios, Fetch).Run the following script to verify all adapter implementations correctly implement this interface:
examples/express-ts/src/__tests__/mocha/product.test.ts (1)
1-1: LGTM! Import path update aligns with test reorganization.The import path change from
../indexto../../indexcorrectly reflects the test file's new location in the directory structure, consistent with the broader test reorganization described in the PR.Verify that the import resolves correctly:
lib/dsl/index.ts (1)
22-23: LGTM! Consider adding JSDoc documentation.The export structure follows TypeScript best practices with proper named exports and
export typesyntax for types. The new wrapper-based testing functionality integrates cleanly with the existing DSL.Consider adding JSDoc comments to document these public API exports for better developer experience:
+/** + * Wraps a test function to enable automatic HTTP request/response capture. + * @see {@link WrappedTestFunction} + */ export { wrapTest, createClient } from "../wrappers" +/** + * Metadata and types for wrapper-based API testing. + */ export type { ApiDocMetadata, WrappedTestFunction } from "../wrappers"Verify that the
../wrappersmodule exports match these imports:examples/express-ts/src/__tests__/jest/product.wrapper.spec.ts (4)
8-19: LGTM!The setup correctly initializes the wrapper-based testing approach with proper state management. The
beforeEachhook ensures test isolation by resetting products.
109-142: LGTM!This CRUD workflow test demonstrates best practices by using dynamic IDs from responses rather than hardcoded values, properly chaining operations, and including comprehensive metadata.
167-193: Verify authentication is actually enforced.These tests include authorization headers but only verify successful responses. They don't validate that authentication is actually required or enforced by the API.
Consider adding negative test cases:
- Verify requests without the auth header are rejected
- Verify invalid tokens are rejected
- Check that the API returns appropriate error codes (401/403)
Example:
apiTest("should reject request without authorization", async () => { const response = await request .post("/api/products") .send({ name: "Unauthorized Product", price: 299.99, category: "Secure", }) expect(response.status).toBe(401) })Also, consider adding metadata to the second test for consistency.
144-165: Verify endpoint and add filtering validation.These tests have two concerns:
They're hitting
/api/products/1(single product endpoint) rather than a list endpoint like/api/products. Query parameters for filtering typically apply to list endpoints, not single-item endpoints.The tests only verify
status 200without validating that filtering actually works (e.g., checking that returned products match the filter criteria).Please verify:
- Should these tests hit
/api/productsinstead of/api/products/1?- If the endpoint is correct, add assertions to validate the filtering behavior
- const response = await request - .get("/api/products/1") - .query({ category: "Electronics", minPrice: 500 }) + const response = await request + .get("/api/products") + .query({ category: "Electronics", minPrice: 500 }) expect(response.status).toBe(200) + // Validate filtering actually works + expect(response.body).toBeInstanceOf(Array) + response.body.forEach(product => { + expect(product.category).toBe("Electronics") + expect(product.price).toBeGreaterThanOrEqual(500) + })examples/express-ts/src/services/productService.ts (2)
8-13: LGTM! State initialization supports test isolation.Converting to
letand adding thenextIdcounter enables proper test cleanup via the newresetProductsmethod. The initial value ofnextId = 3correctly reflects the next ID after the two seeded products.
26-26: Good improvement to use auto-incrementing IDs.Replacing hard-coded IDs with
nextId++ensures unique identifiers for each new product. The post-increment operator correctly assigns the current value before incrementing.lib/wrappers/index.ts (1)
53-62: LGTM! Clean export structure.The export organization is clear and follows best practices:
- Main functions (
wrapTest,createClient) are exported separately- Types are properly re-exported from their respective modules
- The structure provides a clean public API surface
Verify that all intended public APIs from the PR objectives are exposed:
examples/express-ts/src/__tests__/mocha/upload.wrapper.test.ts (1)
1-16: LGTM! Clean setup for wrapper-based testing.The imports and setup correctly demonstrate the new wrapper-based testing approach with
wrapTest()andcreateClient.supertest(). The pattern is clear and follows the intended design.examples/express-ts/src/__tests__/mocha/product.wrapper.test.ts (2)
1-20: LGTM! Test setup is well-structured.The imports, wrapper initialization, and
beforeEachhook for resetting product state are properly configured for test isolation.
169-195: LGTM! Authentication header tests demonstrate wrapper capabilities.These tests effectively show how the wrapper captures various authentication and custom headers, which is valuable for API documentation generation.
Note: The tests don't validate actual authentication logic (token validity, authorization checks), but this is acceptable if the focus is on demonstrating header capture for documentation purposes.
lib/wrappers/__tests__/integration.test.ts (2)
22-50: LGTM!The test setup creates a fresh Express app for each test with appropriate mock routes. The beforeEach hook ensures proper test isolation.
52-106: LGTM!The test scenarios effectively demonstrate the wrapper-based API testing approach with meaningful assertions covering success paths, validation errors, multi-step flows, and metadata attachment.
examples/express-ts/src/__tests__/mocha/user.wrapper.test.ts (2)
1-15: LGTM! Clean wrapper setup.The imports and initialization follow the wrapper-based testing pattern correctly. The documentation header clearly explains the purpose of this test file.
84-84: Verify error message consistency.The error message here is
"Invalid credentials"(no period), while the validation error messages on Lines 42 and 54 are"Username and password are required."(with period). Ensure this inconsistency reflects the actual API behavior, or standardize error message formatting across endpoints.examples/express-ts/src/__tests__/jest/upload.wrapper.spec.ts (1)
1-8: LGTM! Clean setup using the new wrapper-based API.The imports and initial configuration correctly use the new
wrapTestandcreateClientwrappers introduced in this PR, demonstrating the intended usage pattern for wrapper-based testing.examples/express-ts/src/__tests__/jest/user.wrapper.spec.ts (4)
1-13: LGTM!The imports and test setup are clean and follow the wrapper-based testing approach correctly.
15-55: LGTM!The registration endpoint tests appropriately cover the success case and validation failures. The use of
withMetaensures proper documentation generation.
57-85: LGTM!The login endpoint tests cover both successful authentication and error handling appropriately.
115-136: Fix the workflow test logic.The test registers a new user "newuser" (lines 116-119) but then logs in with different credentials "admin"/"admin" (lines 124-127). This breaks the intended workflow of testing that a newly registered user can successfully log in.
Apply this diff to fix the workflow logic:
const loginResponse = await request.post("/api/user/login").send({ - username: "admin", - password: "admin", + username: "newuser", + password: "newpassword", })Additionally, consider whether the hardcoded user ID "123" (line 132) will work correctly in your test environment, or if you should use the registered user's ID instead.
lib/wrappers/adapters/index.ts (2)
22-70: LGTM! Clean factory pattern with comprehensive documentation.The factory structure is well-organized with clear separation between adapter types. The JSDoc examples are helpful for users.
81-87: LGTM! Proper export structure with type safety.The exports correctly distinguish between type-only exports (
export type) and runtime exports, following TypeScript best practices for module boundaries.
examples/express-ts/src/__tests__/mocha/product.wrapper.test.ts
Outdated
Show resolved
Hide resolved
|
@wnghdcjfe @PENEKhun Some rollbacks have been made. Please leave a review! |
|
mocha_test.zip |
PENEKhun
added
the
Holding
High-Order Function Wrapper for Automatic API Documentation Generation
Overview
Implement high-order function wrapper for Jest/Mocha to capture HTTP requests/responses automatically for API documentation generation.
Core Features
Minimal Code Changes
Key Components
Advanced Features
Implementation
Core Files Added
lib/wrappers/wrapTest.ts- Main wrapper functionlib/wrappers/core/CaptureContext.ts- Context managementlib/wrappers/adapters/- HTTP client adapterslib/wrappers/types.ts- TypeScript definitionsexamples/express-ts/src/__tests__/*.wrapper.test.ts- Test examplesTest Coverage
Usage Examples
Basic Usage
With Metadata
File Upload
Multiple HTTP Clients
Migration Guide
From Existing Supertest
Benefits
Known Issues & TODOs
This is a draft implementation for issue #241. Further review and testing required before production use.
Related: #241, #247
Summary by CodeRabbit