Bug 17307 – KURL (and the rest of WebCore) needs unit testing

• Status: RESOLVED
• Resolution: INVALID
• Component: New Bugs

[Eric Seidel (no email)]

KURL (and the rest of WebCore) needs unit testing

I copied the unit tests from Google's open source URL parsing library google-url into WebCore tonight.  I then built a clean-room implementation of enough of their testing framework (gunit.h) (although I have access to the source of such at work, I've not yet taken advantage of such).

This patch is incomplete, since in order to actually test anything in WebCore we have to expose a bunch of extra symbols from JavaScriptCore and WebCore.  For my local copy, I just exposed all symbols to make it build.

[Eric Seidel (no email)]

Created attachment 19062 [details]
First (hackish) pass at adding unit testing to WebCore

 .../JavaScriptCore.xcodeproj/project.pbxproj       |    3 +
 WebCore/Configurations/Unit Tests.xcconfig         |    5 +
 WebCore/WebCore.xcodeproj/project.pbxproj          |  126 ++++++++
 WebCore/unit-tests/TestCase.cpp                    |   29 ++
 WebCore/unit-tests/TestCase.h                      |   71 ++++
 WebCore/unit-tests/TestController.cpp              |   50 +++
 WebCore/unit-tests/TestController.h                |   47 +++
 WebCore/unit-tests/main.cpp                        |   18 +
 WebCore/unit-tests/platform/KURL_tests.cpp         |  339 ++++++++++++++++++++
 9 files changed, 688 insertions(+), 0 deletions(-)

[Eric Seidel (no email)]

Btw, for any of those of you curious as to what the actual output of this is:
FAIL: resolve_cases[i].expected_valid == output.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:187)
.
FAIL: resolve_cases[i].expected_valid == outputw.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:194)
.
FAIL: resolve_cases[i].expected_valid == output.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:187)
.
FAIL: resolve_cases[i].expected_valid == outputw.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:194)
.
FAIL: resolve_cases[i].expected_valid == output.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:187)
.
FAIL: resolve_cases[i].expected_valid == outputw.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:194)
.
FAIL: resolve_cases[i].expected_valid == output.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:187)
.
FAIL: resolve_cases[i].expected_valid == outputw.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:194)
.
FAIL: resolve_cases[i].expected_valid == output.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:187)

FAIL: resolve_cases[i].expected == output.spec() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:188)

FAIL: resolve_cases[i].expected_valid == outputw.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:194)

FAIL: resolve_cases[i].expected == outputw.spec() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:195)

FAIL: resolve_cases[i].expected_valid == output.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:187)
.
FAIL: resolve_cases[i].expected_valid == outputw.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:194)
.
FAIL: resolve_cases[i].expected_valid == output.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:187)

FAIL: resolve_cases[i].expected == output.spec() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:188)

FAIL: resolve_cases[i].expected_valid == outputw.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:194)

FAIL: resolve_cases[i].expected == outputw.spec() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:195)
....
FAIL: replace_cases[i].expected == url.spec() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:252)

FAIL: replace_cases[i].expected == url.spec() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:252)
..
FAIL: ip_tests[i].expected_ip == url.HostIsIPAddress() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:290)
....
FAIL: url.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:79)
..........
FAIL: "q=a" == url.query() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:93)
.
FAIL: url_1.DomainIs(google_domain) (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:298)
.
FAIL: url_3.DomainIs(google_domain) (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:304)
..
FAIL: url_6.DomainIs(".com.") (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:313)
......
FAIL: cases[i].expected == path_request (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:271)
.
FAIL: cases[i].expected == path_request (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:271)
.
FAIL: "http://google.com:foo/" == url.possibly_invalid_spec() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:150)
...
FAIL: "google.com" == url.host() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:155)

FAIL: "foo" == url.port() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:156)
.
FAIL: "/" == url.path() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:158)
...
FAIL: cases[i].expected == empty_path.spec() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:212)

FAIL: cases[i].expected == empty_path.spec() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:212)
......
FAIL: "" == url.port() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:106)
....
FAIL: "http://www.google.com/asdf" == url_1.spec() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:332)

FAIL: "http://www.google.com/foo" == url_2.spec() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:336)

FAIL: url2.is_valid() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:117)
........
FAIL: "q=a" == url2.query() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:127)
.......
FAIL: "" == invalid2.port() (/Stuff/Projects/WebKit/WebCore/unit-tests/platform/KURL_tests.cpp:139)
....

[Darin Fisher (:fishd, Google)]

> we have to expose a bunch of extra symbols from JavaScriptCore and WebCore

perhaps it would be better to link to the underlying object files instead of trying to export more symbols.  this kind of thing can be much easier to arrange if an intermediate static library is created.  then you can link that static library into your unit tests executable as well as into the final dylib, dll, or whatever.

[Eric Seidel (no email)]

Created attachment 19920 [details]
Fix embed.src and object.data updating

 LayoutTests/ChangeLog                              |   13 +++++
 .../dynamic-frame-load-after-layout-expected.txt   |   29 ++++++++++++
 .../frames/dynamic-frame-load-after-layout.html    |   27 +++++++++++
 .../dynamic-frame-replace-after-load-expected.txt  |   34 ++++++++++++++
 .../frames/dynamic-frame-replace-after-load.html   |   48 ++++++++++++++++++++
 WebCore/ChangeLog                                  |   17 +++++++
 WebCore/html/HTMLEmbedElement.cpp                  |   17 ++++++-
 WebCore/html/HTMLEmbedElement.h                    |    1 +
 WebCore/html/HTMLObjectElement.cpp                 |    6 ++-
 9 files changed, 188 insertions(+), 4 deletions(-)

[Eric Seidel (no email)]

Comment on attachment 19920 [details]
Fix embed.src and object.data updating

wrong bug!

[Darin Fisher (:fishd, Google)]

Please note:  WebKit/chromium/tests/KURLTests.cpp

(That is surely not in the ideal location for a WebCore unit test though.)

[Anne van Kesteren]

We no longer use KURL.