test: add cctest for native URL class

[jasnell]

Adds a cctest for the new native URL class

Depends on: #11801 and #11956

/cc @bmeck@TimothyGu

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)

url, test

[joyeecheung]

Nit: Why not CHECK_EQ(simple.protocol(), "https:");?

[addaleax]

I’d just put this once at the top level instead of having it in each test, tbh

[jasnell]

Updated! :-)

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

[danbev]

Just wondering for future reference if it is preferred to use CHECK_EQ instead of EXPECT_EQ?
The difference seems to be that CHECK_EQ will abort straight away without allowing for the rest of the tests to be run, whereas EXPECT_EQ which is non-fatal and would allow for the rest of the tests to be run and report the errors afterwards.

For example, using CHECK_EQ and forcing a check to fail:

[ RUN ] URLTest.Simple Assertion failed: ((simple.protocol()) == ("httpsss")), function TestBody, file ../test/cctest/test_url.cc, line 23. make: *** [cctest] Abort trap: 6 

Compared to EXPECT_EQ, and forcing a the expectation to fail:

[ RUN ] URLTest.Simple ../test/cctest/test_url.cc:24: Failure Value of: "httpss:" Expected: simple.protocol() Which is: "https:" [ FAILED ] URLTest.Simple (13 ms) ... [ PASSED ] 36 tests. [ FAILED ] 1 test, listed below: [ FAILED ] URLTest.Simple 

[jasnell]

Good point. I'd actually forgotten that EXPECT_EQ existed :-) I'll switch it up to use that instead.

[TimothyGu]

Is there a reason for this change?

[jasnell]

Yes, when node_url.h is included in the cctest/test_url.h, a compile error occurs because the url_error_cb_args is defined multiple times. I had to make similar changes to the other structs defined in this file. It would likely be good to refactor the node_url.h file a bit if we intend for it to be generally includable in more than just node_url.cc.

[jasnell]

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

[jasnell]

CI failures are unrelated.

[jasnell]

Landed in 51b007a

[addaleax]

@jasnell Reopened by accident?

[jasnell]

Yep lol.

[MylesBorins]

This needs a manual backport to v7.x

[aqrln]

@MylesBorins it will cherry-pick cleanly after #12321.