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: test resolveObject with an empty path #11811

Closed
wants to merge 1 commit into from
Closed

test: test resolveObject with an empty path #11811

wants to merge 1 commit into from

Conversation

watilde
Copy link
Member

@watilde watilde commented Mar 12, 2017

Add a case to test an URL object that has no path at all for url.resolveObject.

This test increases the coverage of lib/url.js:

The following lines will be checked:

  • node/lib/url.js

    Lines 846 to 849 in 055482c

    if (!srcPath.length) {
    // no path at all. easy.
    // we've already handled the other stuff above.
    result.pathname = null;

  • node/lib/url.js

    Lines 853 to 858 in 055482c

    } else {
    result.path = null;
    }
    result.href = result.format();
    return result;
    }

This PR was part of #11395 that I created, and I separated a non-breaking change commit from #11395 to this PR. I will update #11395 later.

Checklist
  • make -j4 test
  • tests is included
  • 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 Mar 12, 2017
@watilde watilde added the url Issues and PRs related to the legacy built-in url module. label Mar 12, 2017
@watilde
Copy link
Member Author

watilde commented Mar 12, 2017

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

@watilde
test: test resolveObject with an empty path
069fa16 
Add a case to test an URL object that has no path at all
for `url.resolveObject`.
@watilde watilde changed the title url: test resolveObject with an empty path test: test resolveObject with an empty path Mar 12, 2017
@watilde
Copy link
Member Author

watilde commented Mar 12, 2017

Updated the name of a subsystem to test from url in a commit message and this issue title.

@watilde
Copy link
Member Author

watilde commented Mar 12, 2017

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

@watilde
Copy link
Member Author

watilde commented Mar 13, 2017

Same with this: #11812 (comment). I will rerun the CI.

@watilde
Copy link
Member Author

watilde commented Mar 13, 2017

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

@watilde
Copy link
Member Author

watilde commented Mar 14, 2017

Landed in 92bcc13

@watilde watilde closed this Mar 14, 2017
@watilde watilde deleted the feature/test-url-no-path branch March 14, 2017 19:47
watilde added a commit that referenced this pull request Mar 14, 2017
@watilde
test: test resolveObject with an empty path
92bcc13 
Add a case to test an URL object that has no path at all
for `url.resolveObject`.

PR-URL: #11811
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Mar 20, 2017
@watilde @italoacasas
test: test resolveObject with an empty path
2df662c 
Add a case to test an URL object that has no path at all
for `url.resolveObject`.

PR-URL: nodejs#11811
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@italoacasas italoacasas mentioned this pull request Mar 20, 2017
Merged
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
@watilde @jungx098
test: test resolveObject with an empty path
a3811c4 
Add a case to test an URL object that has no path at all
for `url.resolveObject`.

PR-URL: nodejs#11811
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joyeecheung joyeecheung joyeecheung approved these changes

@hiroppy hiroppy hiroppy approved these changes

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell jasnell approved these changes

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

Successfully merging this pull request may close these issues.
7 participants
@watilde @jasnell @cjihrig @hiroppy @joyeecheung @MylesBorins @nodejs-github-bot