Add Jest unit and integration tests for utils, controller, and routes#352
Add Jest unit and integration tests for utils, controller, and routes#352Sran012 wants to merge 3 commits intozhravan:mainfrom
Conversation
|
@Sran012 is attempting to deploy a commit to the shravan20's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds Jest testing infrastructure and multiple tests, updates devDependencies and npm test scripts, changes frontend react-scripts spec to a caret range, and makes one async change in quotesService plus a whitespace edit in src/app.js. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant QuotesService
participant CardTemplate
Client->>QuotesService: request getQuote(params)
Note right of QuotesService #D3E4CD: validate inputs, prepare template
QuotesService->>CardTemplate: generateTemplate(template)
activate CardTemplate
CardTemplate-->>QuotesService: (promise) SVG string
deactivate CardTemplate
QuotesService-->>Client: return SVG
Note left of Client #F0F4FF: response contains image/svg+xml headers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
package.json (1)
27-30: Add a CI-friendly test script.
Consider a dedicated CI script to avoid watch mode defaults and ensure a non-interactive run with coverage."scripts": { + "test:ci": "jest --ci --coverage --reporters=default --reporters=jest-junit",tests/setup.js (1)
2-11: Restore console after tests to avoid leaking mocks across environments.
Overriding global.console is fine; add teardown to reinstate originals so other tooling isn’t affected.-process.env.NODE_ENV = 'test'; - -global.console = { - ...console, - log: jest.fn(), - debug: jest.fn(), - info: jest.fn(), - warn: jest.fn(), - error: jest.fn(), -}; +process.env.NODE_ENV = 'test'; + +const originalConsole = { ...console }; +global.console = { + ...console, + log: jest.fn(), + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), +}; + +afterAll(() => { + // Restore the original console to avoid affecting other tools/runners + // that may execute after Jest in the same process. + // eslint-disable-next-line no-global-assign + global.console = originalConsole; +});tests/integration/app.routes.test.js (2)
13-20: Silence HTTP logs in tests.
morgan writes to stdout and can spam CI logs. No-op it in tests.+jest.mock('morgan', () => () => (_req, _res, next) => next()); ... - const morgan = require('morgan'); const routes = require('../../src/api/routes/quotes-router'); const cors = require('cors'); app.use(cors()); app.use(bodyParser.json()); app.use(bodyParser.urlencoded({ extended: false })); - app.use(morgan('dev')); + // morgan is mocked above to a no-op
24-31: Also assert cache headers to mirror controller contract.
Add header checks to catch regressions end-to-end.const res = await request(server).get('/quote'); expect(res.statusCode).toBe(200); expect(res.headers['content-type']).toMatch(/image\/svg\+xml/); + expect(res.headers['cache-control']).toBe('no-cache,max-age=0,no-store,s-maxage=0,proxy-revalidate'); + expect(res.headers['pragma']).toBe('no-cache'); + expect(res.headers['expires']).toBe('-1'); const body = res.text ?? (Buffer.isBuffer(res.body) ? res.body.toString() : undefined); expect(body).toBe('<svg>integration</svg>');tests/api/controllers/quotesController.test.js (1)
15-27: Add call assertion and error-path coverage.
Strengthen the test by verifying the service is invoked and by covering the catch branch.it('sets headers and sends SVG response', async () => { const req = { query: {} }; const res = createMockRes(); await quoteController(req, res); + const { getQuote } = require('../../../src/api/services/quotesService'); + expect(getQuote).toHaveBeenCalledTimes(1); expect(res.setHeader).toHaveBeenCalledWith('Content-Type', 'image/svg+xml'); expect(res.header).toHaveBeenCalledWith('Cache-Control', 'no-cache,max-age=0,no-store,s-maxage=0,proxy-revalidate'); expect(res.header).toHaveBeenCalledWith('Pragma', 'no-cache'); expect(res.header).toHaveBeenCalledWith('Expires', '-1'); expect(res.send).toHaveBeenCalledWith('<svg>mock</svg>'); }); + + it('handles service errors', async () => { + const { getQuote } = require('../../../src/api/services/quotesService'); + getQuote.mockRejectedValueOnce(new Error('boom')); + const req = { query: {} }; + const res = createMockRes(); + await quoteController(req, res); + expect(res.send).toHaveBeenCalledWith(expect.objectContaining({ name: expect.any(String), message: 'boom' })); + });tests/utils/validateUrl.test.js (1)
1-21: Nice coverage of happy-path and GitHub blob transform.
Consider an extra case to ensure raw GitHub URLs are left unchanged.describe('getValidUrl', () => { @@ it('transforms github blob URL to raw URL', async () => { const res = await getValidUrl('https://github.com/user/repo/blob/main/file.json'); expect(res.isValidUrl).toBe(true); expect(res.customQuotesUrl).toBe('https://raw.githubusercontent.com/user/repo/main/file.json'); }); + + it('keeps raw.githubusercontent.com URLs unchanged', async () => { + const url = 'https://raw.githubusercontent.com/user/repo/main/file.json'; + const res = await getValidUrl(url); + expect(res.isValidUrl).toBe(true); + expect(res.customQuotesUrl).toBe(url); + }); });tests/utils/fetchApi.test.js (2)
11-16: Relax the call expectation to avoid coupling toundefinedoptions.
Future changes may pass{}or headers; keep the assertion tolerant.it('returns parsed json on success', async () => { fetch.mockResolvedValue({ json: async () => ({ ok: true }) }); const res = await fetchApi('https://example.com'); expect(res).toEqual({ ok: true }); - expect(fetch).toHaveBeenCalledWith('https://example.com', undefined); + expect(fetch).toHaveBeenCalledWith('https://example.com', expect.anything()); });
18-22: Optional: cover JSON parse failure.
Add a test to ensure errors from response.json propagate as rejections.it('throws on fetch error', async () => { const err = new Error('network'); fetch.mockRejectedValue(err); await expect(fetchApi('https://example.com')).rejects.toThrow('network'); }); + + it('throws when response.json fails', async () => { + fetch.mockResolvedValue({ json: async () => { throw new Error('bad json'); } }); + await expect(fetchApi('https://example.com')).rejects.toThrow('bad json'); + });jest.config.js (2)
3-3: SimplifytestMatchto avoid redundant patterns and future TS/JSX support.- testMatch: ['**/__tests__/**/*.js', '**/?(*.)+(spec|test).js'], + testMatch: ['**/?(*.)+(spec|test).[jt]s?(x)'],
2-12: Consider default mock hygiene to reduce test flakiness.testEnvironment: 'node', + clearMocks: true, + restoreMocks: true,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
frontend/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
frontend/package.json(1 hunks)jest.config.js(1 hunks)package.json(1 hunks)src/app.js(1 hunks)tests/api/controllers/quotesController.test.js(1 hunks)tests/integration/app.routes.test.js(1 hunks)tests/setup.js(1 hunks)tests/utils/fetchApi.test.js(1 hunks)tests/utils/validateUrl.test.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/api/controllers/quotesController.test.js (1)
src/api/controllers/quotesController.js (1)
quoteController(7-74)
tests/integration/app.routes.test.js (1)
src/app.js (6)
express(1-1)app(12-12)bodyParser(4-4)morgan(5-5)routes(3-3)cors(9-9)
🔇 Additional comments (9)
src/app.js (1)
31-33: No-op whitespace change — OK to merge.
No behavioral impact; routing and Swagger wiring remain unchanged.package.json (1)
17-21: Jest + Supertest tooling — LGTM.
Dev dependencies are appropriate for the test suites added in this PR.tests/integration/app.routes.test.js (1)
1-6: Good: service-level mock keeps the test hermetic.
Mocking quotesService.getQuote at module level ensures deterministic SVG output.tests/api/controllers/quotesController.test.js (1)
1-5: Unit isolation looks solid.
Controller is tested with the service mocked; good focus on headers and payload.frontend/package.json (1)
14-14: Resolved — lockfile pins react-scripts@5.0.1
frontend/package-lock.json shows react-scripts resolved to 5.0.1.jest.config.js (4)
11-11: LGTM —setupFilesAfterEnvpath present and conventional.
tests/setup.js present.
1-1: Approved — keep jest.config.js as CommonJSpackage.json "type" is "commonjs", so jest.config.js using module.exports is correct; no change needed.
13-13: Incorrect — keep in testPathIgnorePatterns.
Jest supports the token in testPathIgnorePatterns, so the existing '/frontend/' pattern is valid and more robust than '/frontend/'. (jestjs.io)Likely an incorrect or invalid review comment.
4-8: Adjust collectCoverageFrom to match this repo — don’t add controllers/routes/utils (they don’t exist).jest.config.js currently only collects 'src/**/*.js' and excludes tests only under src; the repo has no controllers/, routes/, or utils/ directories, contains server.js at the repo root, and frontend/ is explicitly ignored by testPathIgnorePatterns. Update coverage to include actual top-level/server files and exclude test files globally.
File: jest.config.js (collectCoverageFrom) — suggested replacement:
collectCoverageFrom: [
'server.js',
'src//*.js',
'!/.test.js',
'!**/.spec.js'
],Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/api/services/quotesService.js (1)
61-62: Inline return to drop the temporary.Minor tidy-up; same behavior and still caught by the try/catch.
- let svg = await cardTemplate.generateTemplate(template); - return svg; + return await cardTemplate.generateTemplate(template);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/api/services/quotesService.js(1 hunks)
🔇 Additional comments (1)
src/api/services/quotesService.js (1)
61-61: Awaiting template generation is correct — confirm non-test change was intentional.generateTemplate is async (src/utils/generateTemplate.js) and getQuote is awaited by the controller (src/api/controllers/quotesController.js:54); awaiting here lets the surrounding try/catch surface rejections and does not break callers. Confirm this production change was intended in a tests-only PR.
2 participants
Summary by CodeRabbit