-
-
Notifications
You must be signed in to change notification settings - Fork 2
Initial tests for middleware functionality. #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
test/middleware/express.test.ts
Outdated
|
|
||
| complexRequest = { | ||
| // complexity should be 10 if 'first' is accounted for. | ||
| // scalars: 1, droid: 1, reviews (4 * (1 Review, 1 episode)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enums are equivalent to scalarn and have a weight of zero, not 1, so this won't work as expected.
test/middleware/express.test.ts
Outdated
| // Send 5 queries of complexity 2. These should all succeed | ||
| middleware(mockRequest as Request, mockResponse as Response, nextFunction); | ||
|
|
||
| // advance the timers by 1 second for the next request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says 1 second and the code says 20ms. I think the code is right but the intention is muddled.
test/middleware/express.test.ts
Outdated
| }); | ||
|
|
||
| test('Uses User IP Address in Redis', async () => { | ||
| const client: RedisClientType = redis.createClient(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this redis client getting into the middleware or accessing the data inside the middleware? I'm don't know enough about redis to understand what is happening here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to flush this out a bit since during middleware initialization the node-redis client would connect to an existing redis instance whereas during testing redis-mock does all operations in memory. This may just need to be dropped from the tests or we can use something like NODE_ENV=TEST to control how the middleware connects to redis during initialization.
Corrected middleware tests and set to skip all middleware tests.
evanmcneely
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think the complexities of some of the test queries will make the tests not behave as as expected. See comments.
Summary
Writes tests for Token Bucket implementation of Express Rate Limiting Middleware.
Sets package type to module
Reconfigures jest using typescript for to support module configuration.
Type of Change
Issues
Evidence