Skip to content

chore: testing updates and tweaks - #107

Merged
nfriedly merged 5 commits into
mainfrom
test-tweaks
Aug 26, 2026
Merged

chore: testing updates and tweaks#107
nfriedly merged 5 commits into
mainfrom
test-tweaks

Conversation

@nfriedly

@nfriedly nfriedly commented Aug 26, 2026

Copy link
Copy Markdown
Member

This is a handful of related test improvements that I am just trying to get out of the way to work on something else:

  • Upgraded to the latest ts-jest dependency because my local tests started failing with jest reading untransformed typescript as if it were javascript, and upgrading ts-jest fixed it
  • Dropped @types/jest in favor of @jest/globals - https://jestjs.io/docs/getting-started#type-definitions recommends one or the other, not both
  • explicitly imported jest object and methods into each test file where they're used (the @jest/globals way) - this makes my editory happy
  • fixed a few tests that were passing a failure message to expect - jest doesn't support this, I'm not sure why it was passing lint before, maybe @types/jest is overly permissive?
  • split one compound expect(A && B) into two standalone expect(A); expect(B) calls
  • fixed a typo I noticed

@nfriedly
nfriedly requested a review from gamemaker1 August 26, 2026 15:26
Comment thread test/library/delay-test.ts Outdated
// Tests the delaying mechanism

import { jest } from '@jest/globals'
import { jest, describe, beforeEach, afterEach, it } from '@jest/globals'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import { jest, describe, beforeEach, afterEach, it } from '@jest/globals'
import { jest, describe, beforeEach, afterEach, it, expect } from '@jest/globals'

@gamemaker1 gamemaker1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made one suggested change to fix the CI failure. Rest looks good.

Since Typescript 7 released, I have been thinking of moving things from jest to vitest (no/minimal code changes required since API is the same) to avoid all this plumbing we have to do for Typescript support. Similarly, we could move from esbuild + dts-bundle-generator to tsdown (reduces a lot of our config mess), since dts-bundle-generator doesn't yet support Typescript 7.

I'll make PRs for each and we'll see how it looks, if you think it is good, we can merge it for this package and the express-rate-limit package as well.

nfriedly and others added 5 commits August 26, 2026 16:33
Fixes #105

The default maxDelayMs is now the largest 32-bit signed integer instead of infinity, and it will throw an error at instantiation time (instead of later on at runtime) if you try to set a value higher than that.

Note: I've been testing a local/offline ai coding setup, and it did most of the changes here. I reviewed it myself already, but please give this one a little extra scrutiny in case there's anything that I missed.
Co-authored-by: Vedant Kulkarni <gamemaker0042@gmail.com>
This is a handful of related test improvements that I am just trying to get out of the way to work on something else:

* Upgraded to the latest ts-jest dependency because my local tests started failing with jest reading untransformed typescript as if it were javascript, and upgrading ts-jest fixed it
* Dropped @types/jest in favor of @jest/globals - https://jestjs.io/docs/getting-started#type-definitions recommends one or the other, not both
* explicitly imported jest object and methods into each test file where they're used (the @jest/globals way) - this makes my editory happy
* fixed a few tests that were passing a failure message to expect - jest doesn't support this, I'm not sure why it was passing lint before, maybe @types/jest is overly permissive?
* split one compound expect(A && B) into two standalone expect(A); expect(B); calls
* fixed a typo I noticed
@nfriedly

Copy link
Copy Markdown
Member Author

Shoot, I thought I had things passing here before I opened the PR. Maybe it was the merge with master. I ended up fixing a formatting glitch in main and then re-basing this branch against it. It took a couple of attempts, but I think I have it correct now.

The other things you mentioned sound good, at least worth looking into.

@nfriedly
nfriedly merged commit 83b528b into main Aug 26, 2026
20 checks 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