FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: improve punycode test coverage by seppevs · Pull Request #11144 · nodejs/node · GitHub

/ node Public

test: improve punycode test coverage - #11144

Closed
seppevs wants to merge 1 commit into
nodejs:masterfrom
seppevs:test_punycode_map_domain
ConversationCommits (1)ChecksFiles changed
Closed

test: improve punycode test coverage#11144
seppevs wants to merge 1 commit into
nodejs:masterfrom
seppevs:test_punycode_map_domain

Conversation

seppevs commented Feb 3, 2017

Copy link
Copy Markdown
Contributor

Adds two additional tests for mapDomain function in punycode.js.

When an email address is given to the toASCII() and toUnicode() functions, only the parts before the '@' character should be encoded/decoded.

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

N/A

test: improve punycode test coverage
3b05c2b 
Adds two additional tests for mapDomain function in punycode.js.
When an email address is given to the toASCII() and toUnicode() functions, only the parts before the '@' character should be encoded/decoded.
nodejs-github-bot added the test Issues and PRs related to the tests. label Feb 3, 2017
hiroppy added the punycode Issues and PRs related to the punycode module bundled in Node.js. label Feb 3, 2017

hiroppy commented Feb 5, 2017

Copy link
Copy Markdown
Member

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

thefourtheye approved these changes Feb 6, 2017
View reviewed changes

thefourtheye left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM if the CI is happy.

seppevs commented Feb 6, 2017

Copy link
Copy Markdown
Contributor Author

The CI fails, but I don't think it's caused by my changes?

joyeecheung commented Feb 6, 2017

Copy link
Copy Markdown
Member

Refs: #10990 (comment)

@addaleax @mathiasbynens If we move punycode.js to deps, should we upstream the tests?

joyeecheung commented Feb 6, 2017

Copy link
Copy Markdown
Member

BTW: CI failure looks unrelated.

mathiasbynens commented Feb 6, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

@joyeecheung There’s an upstream test for this already: https://github.com/bestiejs/punycode.js/blob/9aeca525bba478206c6e1b5501e063f3db7bda7f/tests/tests.js#L211-L215 If I’m missing something and this patch somehow tests a different code path, please submit a PR to https://github.com/bestiejs/punycode.js as well.

seppevs commented Feb 7, 2017

Copy link
Copy Markdown
Contributor Author

@mathiasbynens : the upstream test seems to cover the same code.

jasnell commented Feb 7, 2017

Copy link
Copy Markdown
Member

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

jasnell commented Feb 11, 2017

Copy link
Copy Markdown
Member

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

jasnell pushed a commit that referenced this pull request Feb 13, 2017
test: improve punycode test coverage
c6238e2 
Adds two additional tests for mapDomain function in punycode.js.
When an email address is given to the toASCII() and toUnicode()
functions, only the parts before the '@' character should be
encoded/decoded.

PR-URL: #11144
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

jasnell commented Feb 13, 2017

Copy link
Copy Markdown
Member

Landed in c6238e2

jasnell closed this Feb 13, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Feb 14, 2017
test: improve punycode test coverage
6f866ae 
Adds two additional tests for mapDomain function in punycode.js.
When an email address is given to the toASCII() and toUnicode()
functions, only the parts before the '@' character should be
encoded/decoded.

PR-URL: nodejs#11144
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
italoacasas mentioned this pull request Feb 14, 2017
Merged
jasnell pushed a commit that referenced this pull request Mar 7, 2017
test: improve punycode test coverage
319bf43 
Adds two additional tests for mapDomain function in punycode.js.
When an email address is given to the toASCII() and toUnicode()
functions, only the parts before the '@' character should be
encoded/decoded.

PR-URL: #11144
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
jasnell pushed a commit that referenced this pull request Mar 7, 2017
test: improve punycode test coverage
afa1219 
Adds two additional tests for mapDomain function in punycode.js.
When an email address is given to the toASCII() and toUnicode()
functions, only the parts before the '@' character should be
encoded/decoded.

PR-URL: #11144
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
test: improve punycode test coverage
c539325 
Adds two additional tests for mapDomain function in punycode.js.
When an email address is given to the toASCII() and toUnicode()
functions, only the parts before the '@' character should be
encoded/decoded.

PR-URL: #11144
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins mentioned this pull request Mar 9, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
test: improve punycode test coverage
8629c95 
Adds two additional tests for mapDomain function in punycode.js.
When an email address is given to the toASCII() and toUnicode()
functions, only the parts before the '@' character should be
encoded/decoded.

PR-URL: #11144
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins mentioned this pull request Mar 9, 2017
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
+1 more reviewer
thefourtheye thefourtheye approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

punycode Issues and PRs related to the punycode module bundled in Node.js. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL