test(auth): add unit tests for JwtValidator#10
Conversation
- Added comprehensive tests for `JwtValidator` in `packages/auth/src/jwt.test.ts`. - Mocked `jose` and `JwtProviderManager` to isolate tests. - Added `vitest` to root `devDependencies` to fix test execution. - Verified tests pass and catch regressions. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive unit tests for the JwtValidator class in the @orch/auth package, improving testability and ensuring the JWT validation logic works correctly across various scenarios.
Changes:
- Added
packages/auth/src/jwt.test.tswith 9 test cases covering happy path, error handling, caching, and edge cases - Added
vitest^3.2.4 to rootpackage.jsondevDependencies to enable running tests from the workspace root - Updated
pnpm-lock.yamlto reflect the new vitest dependency and its transitive dependencies
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/auth/src/jwt.test.ts | New test file with comprehensive unit tests for JwtValidator including mocking of jose library and JwtProviderManager |
| package.json | Added vitest ^3.2.4 to root devDependencies for workspace-level test execution |
| pnpm-lock.yaml | Lockfile updates reflecting vitest addition and dependency resolution changes |
| packages/shared/tsconfig.tsbuildinfo | Build artifact update from TypeScript compilation with updated dependencies |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('should throw AUTH_FAILED if provider is not found', async () => { | ||
| vi.mocked(jose.decodeJwt).mockReturnValue({ iss: 'unknown-issuer' }); | ||
| vi.mocked(mockProviderManager.findByIssuer).mockReturnValue(undefined); | ||
|
|
||
| await expect(validator.validate(validToken)).rejects.toThrow(OrchestratorError); | ||
| await expect(validator.validate(validToken)).rejects.toThrow('is not a registered provider'); | ||
|
|
||
| try { | ||
| await validator.validate(validToken); | ||
| } catch (err: any) { | ||
| expect(err.code).toBe(ErrorCode.AUTH_FAILED); | ||
| } | ||
| }); |
There was a problem hiding this comment.
This test calls validator.validate(validToken) three times (lines 99, 100, and 103), which is inefficient and could potentially mask issues. Consider restructuring the test to call the validation method only once and make all assertions on the caught error, similar to the pattern used in other error tests in this file (e.g., lines 172-179).
packages/auth/src/jwt.test.ts
Outdated
| @@ -0,0 +1,209 @@ | |||
| import { describe, it, expect, beforeEach, vi, type Mock } from 'vitest'; | |||
There was a problem hiding this comment.
This test file is missing a header comment describing its purpose. Other test files in this package include a header comment following the pattern: /** * Tests for @orch/auth/[module] — [description] */. Consider adding a similar header comment at the top of this file for consistency.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
|
@Psyborgs-git I've opened a new pull request, #12, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Psyborgs-git <49641518+Psyborgs-git@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
test(auth): address review comments on JwtValidator unit tests
This PR adds unit tests for
JwtValidatorinpackages/auth.It includes:
packages/auth/src/jwt.test.ts.joselibrary andJwtProviderManager.package.jsonto includevitestindevDependencies, enablingpnpm testto run.This addresses the "Missing Tests for JWT Validator" task and improves overall testability.
PR created automatically by Jules for task 12249440911468361910 started by @Psyborgs-git