test: add failing url parse tests as known_issue

[jasnell]

Pull Request check-list

• Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
  this change (including linting)?
• Is the commit message formatted according to [CONTRIBUTING.md][0]?
• If this change fixes a bug (or a performance problem), is a regression
  test (or a benchmark) included?
• Is a documentation update included (if this change modifies
  existing APIs, or introduces new ones)?

Affected core subsystem(s)

url, test

Description of change

url resolve and parse do not currently adhere to the same url spec parsing rules that browsers use, which leads to some issues. This addition to test/known_issues creates a set of tests based on the w3c/whatwg test suite from https://github.com/w3c/web-platform-tests/tree/master/url

[cjihrig]

Does all of this need to be logged for every test? What if it was only printed for failing tests?

[jasnell]

No, it doesn't. I added that in largely to highlight the differences. I can
switch it to only output on failing tests.
On Mar 24, 2016 5:27 AM, "Colin Ihrig" [email protected] wrote:

In test/known_issues/test-url-parse-conformance.js
#5885 (comment):

• username = parsed.auth ? parsed.auth.split(':', 2)[0] : '';
• password = parsed.auth ? parsed.auth.split(':', 2)[1] : '';
• assert.equal(test.username, username);
• assert.equal(test.password, password);
• assert.equal(test.host, parsed.host);
• assert.equal(test.hostname, parsed.hostname);
• assert.equal(+test.port, +parsed.port);
• assert.equal(test.pathname, parsed.pathname || '/');
• assert.equal(test.search, parsed.search || '');
• assert.equal(test.hash, parsed.hash || '');
• console.log(\u2713 - ${test.href});
• } catch (err){
• console.log(\u2717 - ${test.href} - ${err.message});
• failed++;
• }
• console.log(Input: ${test.input});

Does all of this need to be logged for every test? What if it was only
printed for failing tests?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/nodejs/node/pull/5885/files/e8229bd3262164bd842f00ec09fe3e533666eec6#r57309357

[cjihrig]

LGTM

[jasnell]

@cjihrig .. updated, PTAL
/cc @nodejs/testing

[sam-github]

@jasnell is there a list of defects?

[jasnell]

#5832 is one. I don't have a compiled list tho.

[sam-github]

I was wondering if you found more things than I reported.

[jasnell]

Well, the test points out that there are a significant number of inconsistencies between our parsing (and relative URL resolution) than what is implemented / expected on the browser side per the WhatWG specs. This test is really meant just to highlight those. I wouldn't necessarily call them defects, per se, but closing the gaps would be good.

[jasnell]

Updated. /cc @cjihrig@mscdex

[mscdex]

LGTM

[cjihrig]

LGTM

[jasnell]

Landed in fa542eb. Thanks @mscdex and @cjihrig !

[MylesBorins]

Added don't land as I'm assuming we would not be chasing down this problem in lts