Skip to content
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

test: fix args passed to strictEqual #21584

Closed

Conversation

aitchkhan
Copy link
Contributor

The third argument passed to asert.strictEqual() displays the message
passed as third argument and does not report the difference between
actual and expected if the test is failing.
Hence, the third argument has been removed.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jun 29, 2018
@mscdex
Copy link
Contributor

mscdex commented Jun 29, 2018

Commit message prefix should just be test: instead of queue-timer test:

The third argument passed to asert.strictEqual() displays the message
passed as third argument and does not report the difference between
actual and expected if the test is failing.
Hence, the third argument has been removed.
@aitchkhan aitchkhan force-pushed the fix-immediate-timer-que-test branch from 9983ffa to b50a6e6 Compare June 29, 2018 05:47
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM.

@Trott
Copy link
Member

Trott commented Jun 29, 2018

@Trott Trott added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. labels Jun 29, 2018
@Trott
Copy link
Member

Trott commented Jun 29, 2018

👍 this comment for fast-tracking.

@mscdex mscdex changed the title queue-timer test: fix args passed to strictEqual test: fix args passed to strictEqual Jun 29, 2018
@aitchkhan
Copy link
Contributor Author

@Trott @addaleax please let me know if there is anything that I have to fix to make the failing check pass?

@Trott
Copy link
Member

Trott commented Jun 29, 2018

Lone CI failure is a known issue that was fixed in the last hour. Running a Resume Build on node-test-pull-request to re-run just the task that failed:

https://ci.nodejs.org/job/node-test-pull-request/15675/

trivikr pushed a commit that referenced this pull request Jul 4, 2018
The third argument passed to asert.strictEqual() displays the message
passed as third argument and does not report the difference between
actual and expected if the test is failing.
Hence, the third argument has been removed.

PR-URL: #21584
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@trivikr
Copy link
Member

trivikr commented Jul 4, 2018

Landed in 24ee745

Congratulations @aitchkhan on your first commit in Node.js core! 🎉🎉🎉

@trivikr trivikr closed this Jul 4, 2018
targos pushed a commit that referenced this pull request Jul 4, 2018
The third argument passed to asert.strictEqual() displays the message
passed as third argument and does not report the difference between
actual and expected if the test is failing.
Hence, the third argument has been removed.

PR-URL: #21584
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@aitchkhan aitchkhan deleted the fix-immediate-timer-que-test branch July 12, 2018 05:52
@targos targos mentioned this pull request Jul 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants