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: assert - fixed assert error messages to match the tests being run #6241

Closed
wants to merge 1 commit into from

Conversation

suryagh
Copy link
Contributor

@suryagh suryagh commented Apr 16, 2016

Checklist
  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change

Few of the assert error messages for the assert tests were not matching the tests in question. Fixed the strings to better reflect the corresponding tests.

@addaleax addaleax added assert Issues and PRs related to the assert subsystem. test Issues and PRs related to the tests. labels Apr 16, 2016
@suryagh suryagh changed the title test: assert - fixed error messages to match the tests in question test: assert - fixed assert error messages to match the tests being run Apr 16, 2016
@Trott
Copy link
Member

Trott commented Apr 17, 2016

@suryagh
Copy link
Contributor Author

suryagh commented Apr 17, 2016

The build failure was due to net connection timeout in an unrelated test.

Is the intermittent connection timeout a known issue with the Jenkins build?

@suryagh
Copy link
Contributor Author

suryagh commented Apr 18, 2016

@Trott how did you trigger the build? Does it need special access to https://ci.nodejs.org/ ? I suspect the timeout is unrelated to the current changes, so wanted to re-try and see if the build goes through.

@jasnell
Copy link
Member

jasnell commented Apr 18, 2016

LGTM

@jasnell
Copy link
Member

jasnell commented Apr 19, 2016

@suryagh ... the failure is a known flaky test.

@santigimeno
Copy link
Member

LGTM

jasnell pushed a commit that referenced this pull request Apr 20, 2016
PR-URL: #6241
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
@jasnell
Copy link
Member

jasnell commented Apr 20, 2016

Landed in aa4d2ae

@jasnell jasnell closed this Apr 20, 2016
@suryagh suryagh deleted the fix branch April 20, 2016 18:17
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
PR-URL: #6241
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
This was referenced Apr 21, 2016
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
PR-URL: #6241
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request Apr 25, 2016
PR-URL: nodejs#6241
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
jasnell pushed a commit that referenced this pull request Apr 26, 2016
PR-URL: #6241
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 17, 2016
PR-URL: #6241
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 18, 2016
PR-URL: #6241
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants