Skip to content

test: add comprehensive unit tests for rate-limiter and url-shortener#14

Merged
CSenshi merged 13 commits intoCSenshi:mainfrom
oniani1:feature/unit-test-coverage
Mar 27, 2026
Merged

test: add comprehensive unit tests for rate-limiter and url-shortener#14
CSenshi merged 13 commits intoCSenshi:mainfrom
oniani1:feature/unit-test-coverage

Conversation

@oniani1
Copy link
Copy Markdown
Contributor

@oniani1 oniani1 commented Mar 25, 2026

Summary

  • Add unit tests for 6 untested rate-limiter files: RuleManagerService, AlgorithmManagerService, ClientIdentifierService, RateLimiterService, RateLimitGuard, and exectRedisScriptSha helper
  • Add unit tests for 4 untested url-shortener files: UrlRepository, ShortenUrl command handler, GetRealUrl query handler, and BatchCounterService
  • Fix broken Jest configs in both apps by converting .ts to .cts (ESM __dirname issue)

135 total tests passing (44 rate-limiter + 91 url-shortener), 0 failures.

Linked Issues

N/A

Checklist

  • Tests added/updated (if relevant)
  • Documentation updated (if relevant)
  • Linting and formatting pass
  • Ready for review

Additional Context

  • All test files follow the existing *.spec.ts convention and sit alongside their source files
  • Tests use Jest with mocked dependencies (no NestJS TestingModule needed for pure unit tests)
  • Jest config fix: .ts.cts conversion matches the pattern already used by rate-limiter's jest.config.cts and aligns with NX's official convert-jest-config-to-cjs migration
  • No existing source files were modified — only new test files and config fixes

@CSenshi
Copy link
Copy Markdown
Owner

CSenshi commented Mar 27, 2026

  1. Remove all 4 jest config changes - jest.config.ts already works on main (existing unit tests prove it). None of the new tests need the .cts conversion.
  2. Remove the redundant guard test - "should propagate RateLimitExceededException from check" tests an impossible path. The guard creates and throws RateLimitExceededException itself based on result.allowed; rateLimiterService.check never throws it. The ECONNREFUSED test above already covers error propagation.

…gation test

The guard itself throws RateLimitExceededException based on result.allowed;
rateLimiterService.check never throws it. Error propagation is already
covered by the ECONNREFUSED test.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@oniani1
Copy link
Copy Markdown
Contributor Author

oniani1 commented Mar 27, 2026

Removed the redundant guard test --rateLimiterService.check returns a result object and the guard itself throws RateLimitExceededException based on result.allowed. The ECONNREFUSED test already covers error propagation.

On the jest configs though -- reverting them back to .ts breaks the url-shortener tests with ReferenceError: __dirname is not defined in ES module scope. Cleared the Nx cache and confirmed it's not a caching issue. The rate-limiter's jest.config.cts is already .cts on main for the same reason, so the .cts conversion seems necessary for any config that uses __dirname. Happy to look into an alternative fix if you'd prefer not to have the .cts files, but reverting them as-is breaks the test runs.

@CSenshi
Copy link
Copy Markdown
Owner

CSenshi commented Mar 27, 2026

LGTM

@CSenshi CSenshi merged commit 024b317 into CSenshi:main Mar 27, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants