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: test for http.request() invalid method error #10080

Closed
wants to merge 1 commit into from
Closed

test: test for http.request() invalid method error #10080

wants to merge 1 commit into from

Conversation

ashtonian
Copy link
Contributor

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change

Adds a test for when an invalid http method is passed into
http.request() to verify an error is thrown, that the
error is the correct type, and the error message is correct.

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 1, 2016
() => { http.request({method: '\0'}); },
(error) => {
if ((error instanceof TypeError)
&& /Method must be a valid HTTP token/.test(error)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The && should go on the previous line.

(error) => {
if ((error instanceof TypeError)
&& /Method must be a valid HTTP token/.test(error)) {
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of returning true inside the if, you can just return the result of the conditional checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you - reads much better.

Adds a test for when an invalid http method is passed into
http.request() to verify an error is thrown, that the
error is the correct type, and the error message is correct.
@ashtonian
Copy link
Contributor Author

Fixed.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM if CI is green

Copy link
Contributor

@evanlucas evanlucas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@gibfahn
Copy link
Member

gibfahn commented Dec 2, 2016

@addaleax addaleax added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Dec 2, 2016
@targos
Copy link
Member

targos commented Dec 3, 2016

Landed in c5ccc50

@targos targos closed this Dec 3, 2016
targos pushed a commit that referenced this pull request Dec 3, 2016
Adds a test for when an invalid http method is passed into
http.request() to verify an error is thrown, that the
error is the correct type, and the error message is correct.

PR-URL: #10080
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
addaleax pushed a commit that referenced this pull request Dec 5, 2016
Adds a test for when an invalid http method is passed into
http.request() to verify an error is thrown, that the
error is the correct type, and the error message is correct.

PR-URL: #10080
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Dec 5, 2016
2 tasks
addaleax pushed a commit to addaleax/node that referenced this pull request Dec 8, 2016
Adds a test for when an invalid http method is passed into
http.request() to verify an error is thrown, that the
error is the correct type, and the error message is correct.

PR-URL: nodejs#10080
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
jmdarling pushed a commit to jmdarling/node that referenced this pull request Dec 8, 2016
Adds a test for when an invalid http method is passed into
http.request() to verify an error is thrown, that the
error is the correct type, and the error message is correct.

PR-URL: nodejs#10080
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@MylesBorins
Copy link
Contributor

This test fails on v4.x. Is that expected behavior? If so please add do-not-land-on-v4.x label

MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
Adds a test for when an invalid http method is passed into
http.request() to verify an error is thrown, that the
error is the correct type, and the error message is correct.

PR-URL: #10080
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants