Skip to content

Conversation

@gerges-beshay
Copy link

@gerges-beshaygerges-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-botnodejs-github-bot added the test Issues and PRs related to the tests. label Oct 26, 2016
@Trott
Copy link
Member

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

@mscdexmscdex 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

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 <[email protected]> Reviewed-By: James M Snell <[email protected]>
@Trott
Copy link
Member

Landed in 758ca8d

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

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 <[email protected]> Reviewed-By: James M Snell <[email protected]>
@evanlucasevanlucas mentioned this pull request Nov 3, 2016
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 <[email protected]> Reviewed-By: James M Snell <[email protected]>
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 <[email protected]> Reviewed-By: James M Snell <[email protected]>
This was referenced Nov 22, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

async_hooksIssues and PRs related to the async hooks subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@gerges-beshay@Trott@jasnell@cjihrig@mscdex@MylesBorins@nodejs-github-bot