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: changes to test/parallel/test-querystring.js #12661

Closed
wants to merge 2 commits into from
Closed

test: changes to test/parallel/test-querystring.js #12661

wants to merge 2 commits into from

Conversation

lukaszewczak
Copy link
Contributor

Change URIError constructor to regular expression in assert.throws and use block-scope for tests that spans multiple statements

Checklist
  • [x ] make -j4 test (UNIX), or vcbuild test (Windows) passes
  • [x ] commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 26, 2017
@hiroppy hiroppy added the querystring Issues and PRs related to the built-in querystring module. label Apr 26, 2017
@vsemozhetbyt
Copy link
Contributor

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

@lpinca
Copy link
Member

lpinca commented Apr 26, 2017

There are a lot of whitespace changes in the diff which can be ignored appending ?w=1 to the URL.
Hope this helps other reviewers who may not know this handy trick.

@lukaszewczak
Copy link
Contributor Author

Hi, this error during test/windows-frame build is quite strange for me, and looks like some temporary connectivity issue, not related to my changes...? Could you please run once again this build process.

java.nio.channels.ClosedChannelException
	at org.jenkinsci.remoting.protocol.NetworkLayer.onRecvClosed(NetworkLayer.java:154)
	at org.jenkinsci.remoting.protocol.impl.NIONetworkLayer.ready(NIONetworkLayer.java:179)
	at org.jenkinsci.remoting.protocol.IOHub$OnReady.run(IOHub.java:721)
	at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:28)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)
Caused: java.io.IOException: Backing channel 'JNLP4-connect connection from 104.130.175.153/104.130.175.153:49164' is disconnected.

@addaleax
Copy link
Member

@lukaszewczak That’s just the CI infrastructure being a bit problematic sometimes, don’t worry about it. :)

@lukaszewczak
Copy link
Contributor Author

Uff :-) Thank you @addaleax for quick replay.

@addaleax
Copy link
Member

addaleax commented May 7, 2017

Landed in 42958d1 and fixed up linter errors while landing, thanks for the PR!

@addaleax addaleax closed this May 7, 2017
addaleax pushed a commit that referenced this pull request May 7, 2017
@lukaszewczak @addaleax
test: refactor test-querystring
42958d1 
- change URIError constructor to regular expression in assert.throws
- use block-scope for tests that spans multiple statements

PR-URL: #12661
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
anchnk pushed a commit to anchnk/node that referenced this pull request May 19, 2017
@lukaszewczak
test: refactor test-querystring
c671c1c 
- change URIError constructor to regular expression in assert.throws
- use block-scope for tests that spans multiple statements

PR-URL: nodejs#12661
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
@jasnell jasnell mentioned this pull request May 11, 2017
Closed
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
gibfahn pushed a commit that referenced this pull request Jun 20, 2017
@lukaszewczak @gibfahn
test: refactor test-querystring
e52f7f8 
- change URIError constructor to regular expression in assert.throws
- use block-scope for tests that spans multiple statements

PR-URL: #12661
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
@lukaszewczak @MylesBorins
test: refactor test-querystring
52b7d5e 
- change URIError constructor to regular expression in assert.throws
- use block-scope for tests that spans multiple statements

PR-URL: #12661
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DavidCai1111 DavidCai1111 DavidCai1111 approved these changes

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
querystring Issues and PRs related to the built-in querystring module. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@lukaszewczak @vsemozhetbyt @lpinca @addaleax @jasnell @cjihrig @DavidCai1111 @hiroppy @gibfahn @nodejs-github-bot