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: use 'strictEqual' instead of 'equal' #9297

Closed
wants to merge 2 commits into from
Closed

test: use 'strictEqual' instead of 'equal' #9297

wants to merge 2 commits into from

Conversation

gerges-beshay
Copy link

@gerges-beshay gerges-beshay commented Oct 26, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Description of change

Use 'strictEqual' instead of 'equal' in 'test-async-wrap-check-providers'.

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 26, 2016
@Trott
Copy link
Member

Trott commented Oct 26, 2016

Can the != on line 39 be replaced with !== while we're at it? Or does that break the test?

@mscdex mscdex added async_wrap async_hooks Issues and PRs related to the async hooks subsystem. labels Oct 26, 2016
@gerges-beshay
Copy link
Author

Oh, I missed that. I made the change and tested it, and it passed.

@gerges-beshay
Copy link
Author

I noticed that some of the checks reported on GitHub as "not successful". Does the tests currently have some flaky issues? If so, is there a ticket already reporting more details on this to look into?

@Trott
Copy link
Member

Trott commented Oct 26, 2016

There are a lot of test/CI issues right now, unfortunately. Failures are very likely to be unrelated to these changes.

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

@gerges-beshay
Copy link
Author

So what is the current process in these cases?

I believe having deterministic tests is vital. I'll work on that as I get more familiar with the code base and tests.

Trott pushed a commit to Trott/io.js that referenced this pull request Oct 28, 2016
* use 'strictEqual' instead of 'equal'
* use '!==' instead of '!='

PR-URL: nodejs#9297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Copy link
Member

Trott commented Oct 28, 2016

Landed in 758ca8d

@Trott Trott closed this Oct 28, 2016
@Trott
Copy link
Member

Trott commented Oct 28, 2016

So what is the current process in these cases?

Collaborators know what known-issues to ignore (hopefully). In the meantime, people are working to fix stuff. If you want to help with that sort of thing specifically, places to look include:

evanlucas pushed a commit that referenced this pull request Nov 3, 2016
* use 'strictEqual' instead of 'equal'
* use '!==' instead of '!='

PR-URL: #9297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 22, 2016
* use 'strictEqual' instead of 'equal'
* use '!==' instead of '!='

PR-URL: #9297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 22, 2016
* use 'strictEqual' instead of 'equal'
* use '!==' instead of '!='

PR-URL: #9297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This was referenced Nov 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants