-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
refactor(tests): migrate from tap to node test runners #107
refactor(tests): migrate from tap to node test runners #107
Conversation
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
The CI got stuck, this is probably the related issue: nodejs/node#49344. In this PR I have added We can remove Thoughts on this? |
Seems redis getting stuck, can you take a closer look? |
Signed-off-by: mateonunez <mateonunez95@gmail.com>
620fd24
to
cc70131
Compare
Green light! ✅ |
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.
lgtm
package.json
Outdated
@@ -5,7 +5,7 @@ | |||
"main": "index.js", | |||
"types": "index.d.ts", | |||
"scripts": { | |||
"test": "standard | snazzy && tap test/*test.js && tsd", | |||
"test": "standard | snazzy && c8 --100 node --test test/*.js && tsd", |
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.
it was test/*test.js
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.
Nice catch!
test/base.test.js
Outdated
@@ -18,16 +20,16 @@ const dummyStorage = { | |||
} | |||
|
|||
let redisClient | |||
before(async (t) => { | |||
beforeEach(async () => { |
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.
before
is still before
, not beforeEach
test/base.test.js
Outdated
redisClient = new Redis() | ||
}) | ||
|
||
teardown(async (t) => { | ||
afterEach(async () => { |
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.
teardown
is after
test/storage-redis.test.js
Outdated
await redisClient.flushall() | ||
}) | ||
describe('storage redis', async () => { | ||
beforeEach(async () => { |
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.
before
instead of beforeEach
const Redis = require('ioredis') | ||
|
||
const createStorage = require('../src/storage') | ||
const { Cache } = require('../') | ||
|
||
let redisClient | ||
before(async (t) => { |
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.
please keep before and after
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.
Added in the suite
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
Signed-off-by: mateonunez <mateonunez95@gmail.com>
2768782
to
d91f0b2
Compare
well done :) |
This PR aims to resolve #98 by migrating the whole codebase of tests that runs server side using the Node Test Runners
test/base.test.js
test/cache.test.js
test/clear.test.js
test/stale.test.js
test/storage-base.test.js
test/storage-memory.test.js
test/storage-redis.test.js
test/transformer.test.js
test/ttl.test.js
test/utils.test.js
I used
tspl
where required by previous tests. Otherwise I kept the default Node assertions.We need to figure out possibly whether to normalize all assertions under
tspl
or proceed to use only if we are going to ensure assertion count.