Create and implement async/sync test helpers #523
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,125 @@ | ||
| 'use strict'; | ||
|
|
||
| const jwt = require('../'); | ||
| const expect = require('chai').expect; | ||
| const sinon = require('sinon'); | ||
|
|
||
| /** | ||
| * Correctly report errors that occur in an asynchronous callback | ||
| * @param {function(err): void} done The mocha callback | ||
| * @param {function(): void} testFunction The assertions function | ||
| */ | ||
| function asyncCheck(done, testFunction) { | ||
| try { | ||
| testFunction(); | ||
| done(); | ||
|
There was a problem hiding this comment. I guess we want this call ( There was a problem hiding this comment. ^ Silly thing, I'm also fine if you leave it as is, just let me know and I'll merge. There was a problem hiding this comment. Both options have different problems. The way the code is currently means that if the By moving the I decided on what is there now, because I would rather Mocha tell the developer that something went wrong, then ignoring the error, or reporting that I think it's fine to keep things this way and try to fix the library later to avoid this problem altogether. |
||
| } | ||
| catch(err) { | ||
| done(err); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Assert that two errors are equal | ||
| * @param e1 {Error} The first error | ||
| * @param e2 {Error} The second error | ||
| */ | ||
| // chai does not do deep equality on errors: https://github.com/chaijs/chai/issues/1009 | ||
| function expectEqualError(e1, e2) { | ||
| // message and name are not always enumerable, so manually reference them | ||
| expect(e1.message, 'Async/Sync Error equality: message').to.equal(e2.message); | ||
| expect(e1.name, 'Async/Sync Error equality: name').to.equal(e2.name); | ||
|
|
||
| // compare other enumerable error properties | ||
| for(const propertyName in e1) { | ||
| expect(e1[propertyName], `Async/Sync Error equality: ${propertyName}`).to.deep.equal(e2[propertyName]); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Base64-url encode a string | ||
| * @param str {string} The string to encode | ||
| * @returns {string} The encoded string | ||
| */ | ||
| function base64UrlEncode(str) { | ||
| return Buffer.from(str).toString('base64') | ||
| .replace(/[=]/g, "") | ||
| .replace(/\+/g, "-") | ||
| .replace(/\//g, "_") | ||
| ; | ||
| } | ||
|
|
||
| /** | ||
| * Verify a JWT, ensuring that the asynchronous and synchronous calls to `verify` have the same result | ||
| * @param {string} jwtString The JWT as a string | ||
| * @param {string} secretOrPrivateKey The shared secret or private key | ||
| * @param {object} options Verify options | ||
| * @param {function(err, token):void} callback | ||
| */ | ||
| function verifyJWTHelper(jwtString, secretOrPrivateKey, options, callback) { | ||
| // freeze the time to ensure the clock remains stable across the async and sync calls | ||
| const fakeClock = sinon.useFakeTimers({now: Date.now()}); | ||
| let error; | ||
| let syncVerified; | ||
| try { | ||
| syncVerified = jwt.verify(jwtString, secretOrPrivateKey, options); | ||
| } | ||
| catch (err) { | ||
| error = err; | ||
| } | ||
| jwt.verify(jwtString, secretOrPrivateKey, options, (err, asyncVerifiedToken) => { | ||
| try { | ||
| if (error) { | ||
| expectEqualError(err, error); | ||
| callback(err); | ||
| } | ||
| else { | ||
| expect(syncVerified, 'Async/Sync token equality').to.deep.equal(asyncVerifiedToken); | ||
| callback(null, syncVerified); | ||
| } | ||
| } | ||
| finally { | ||
| if (fakeClock) { | ||
| fakeClock.restore(); | ||
MitMaro marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Sign a payload to create a JWT, ensuring that the asynchronous and synchronous calls to `sign` have the same result | ||
| * @param {object} payload The JWT payload | ||
| * @param {string} secretOrPrivateKey The shared secret or private key | ||
| * @param {object} options Sign options | ||
| * @param {function(err, token):void} callback | ||
| */ | ||
| function signJWTHelper(payload, secretOrPrivateKey, options, callback) { | ||
| // freeze the time to ensure the clock remains stable across the async and sync calls | ||
| const fakeClock = sinon.useFakeTimers({now: Date.now()}); | ||
| let error; | ||
| let syncSigned; | ||
| try { | ||
| syncSigned = jwt.sign(payload, secretOrPrivateKey, options); | ||
| } | ||
| catch (err) { | ||
| error = err; | ||
| } | ||
| jwt.sign(payload, secretOrPrivateKey, options, (err, asyncSigned) => { | ||
| fakeClock.restore(); | ||
| if (error) { | ||
| expectEqualError(err, error); | ||
| callback(err); | ||
| } | ||
| else { | ||
| expect(syncSigned, 'Async/Sync token equality').to.equal(asyncSigned); | ||
| callback(null, syncSigned); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| module.exports = { | ||
| asyncCheck, | ||
| base64UrlEncode, | ||
| signJWTHelper, | ||
| verifyJWTHelper, | ||
| }; | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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's a bit weird to use this, however, it is better than the try-catch, and more stable than simply... adding the asserts. So, good, let's keep this solution for now until we find a better one.
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.
Long term, promises solves this problem and that appears to be direction that the JavaScript/Node.js community is moving. But that would mean converting the project to not use callbacks, which would be well outside the scope of fixing the tests.