Skip to content

Commit

Permalink
test: Remove unnecessary asserion messages in test-crypto-hash.js
Browse files Browse the repository at this point in the history
This commit improves asserion messages in parallel/test-crypto-hash.js.
Instead of just simple string literal, messages are changed to also
include values used in assertion, which should improve debugging
in case of errors.

PR-URL: #18984
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
  • Loading branch information
pgrzesik authored and BridgeAR committed Mar 11, 2018
1 parent ebfa8b1 commit 0e35683
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions test/parallel/test-crypto-hash.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@ assert.deepStrictEqual(
);

// stream interface should produce the same result.
assert.deepStrictEqual(a5, a3, 'stream interface is consistent');
assert.deepStrictEqual(a6, a3, 'stream interface is consistent');
assert.notStrictEqual(a7, undefined, 'no data should return data');
assert.notStrictEqual(a8, undefined, 'empty string should generate data');
assert.deepStrictEqual(a5, a3);
assert.deepStrictEqual(a6, a3);
assert.notStrictEqual(a7, undefined);
assert.notStrictEqual(a8, undefined);

// Test multiple updates to same hash
const h1 = crypto.createHash('sha1').update('Test123').digest('hex');
const h2 = crypto.createHash('sha1').update('Test').update('123').digest('hex');
assert.strictEqual(h1, h2, 'multipled updates');
assert.strictEqual(h1, h2);

// Test hashing for binary files
const fn = fixtures.path('sample.png');
Expand Down

3 comments on commit 0e35683

@tniessen
Copy link
Member

@tniessen tniessen commented on 0e35683 Mar 11, 2018

Choose a reason for hiding this comment

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

The commit message should have followed the commit message guidelines, there are typos and the description does not match the actual change.

@BridgeAR
Copy link
Member

Choose a reason for hiding this comment

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

It does. We accept longer names even though it is nicer to keep them short.

@Trott
Copy link
Member

@Trott Trott commented on 0e35683 Mar 11, 2018

Choose a reason for hiding this comment

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

@BridgeAR Remove (instead of remove) might be what @tniessen is talking about. (Also, the misspelling of assertion, but that's a simple typo and not a commit message formatting issue.)

Please sign in to comment.