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.

Sign up for GitHub

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

Jump to bottom

test: replace equal with strictEqual #32727

Closed
wants to merge 1 commit into from
Closed

test: replace equal with strictEqual #32727

wants to merge 1 commit into from

Conversation

JesuHrz
Copy link
Contributor

@JesuHrz JesuHrz commented Apr 8, 2020 •
edited
Loading

This PR replaces the functions equal with strictEqual

  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 8, 2020
addaleax
addaleax approved these changes Apr 8, 2020
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

LGTM but note that assert.strict.equal === assert.strictEqual, i.e. the assertions work the same way as before

@JesuHrz
Copy link
Contributor Author

JesuHrz commented Apr 8, 2020 •
edited
Loading

@addaleax, it's correct, but I wanted to make this change already at the end of the file, assert.strictEqual is being used, so I made this change so that whole test uses this method

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 14, 2020
@nodejs-github-bot
Copy link
Collaborator

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

addaleax pushed a commit that referenced this pull request Apr 15, 2020
@JesuHrz @addaleax
test: replace equal with strictEqual
9c25ab1 
PR-URL: #32727
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleax
Copy link
Member

Landed in 9c25ab1

@addaleax addaleax closed this Apr 15, 2020
MylesBorins pushed a commit that referenced this pull request Apr 17, 2020
@JesuHrz @MylesBorins
test: replace equal with strictEqual
e18a40e 
PR-URL: #32727
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
@JesuHrz @targos
test: replace equal with strictEqual
f14a62a 
PR-URL: nodejs#32727
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
BridgeAR pushed a commit that referenced this pull request Apr 28, 2020
@JesuHrz @BridgeAR
test: replace equal with strictEqual
46cafad 
PR-URL: #32727
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Apr 28, 2020
Merged
targos pushed a commit that referenced this pull request Apr 28, 2020
@JesuHrz @targos
test: replace equal with strictEqual
96c9311 
PR-URL: #32727
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@targos targos mentioned this pull request May 2, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Flarna Flarna Flarna approved these changes

@MrHeer MrHeer MrHeer approved these changes

@cjihrig cjihrig cjihrig approved these changes

@addaleax addaleax addaleax approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@JesuHrz @nodejs-github-bot @addaleax @cjihrig @MrHeer @Flarna