Bug 37480 - Add query string tests from Google URL
Summary: Add query string tests from Google URL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-12 19:54 PDT by Daniel Bates
Modified: 2010-04-13 00:40 PDT (History)
3 users (show)

See Also:


Attachments
Layout Test (5.81 KB, patch)
2010-04-12 19:59 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2010-04-12 19:54:57 PDT
We should implement at least all of the non-encoding specific query string tests from <http://code.google.com/p/google-url/source/browse/trunk/src/url_canon_unittest.cc#1016>.
Comment 1 Daniel Bates 2010-04-12 19:59:53 PDT
Created attachment 53214 [details]
Layout Test
Comment 2 Adam Barth 2010-04-12 20:23:15 PDT
Comment on attachment 53214 [details]
Layout Test

+<script src="resources/utilities.js"></script>

I don't think we can edit these files because they are generated from the make-test-wrappers script.

+  ["as#df", "as%23df"],

I don't think this result is correct, but I agree that we should just literally land what's in the unit tests.  We can do a separate pass to understand the different behaviors here.

Review- because we can't change the generated files.  I think it's ok to copy/paste that small function between tests.
Comment 3 Daniel Bates 2010-04-12 20:28:59 PDT
(In reply to comment #2)
> (From update of attachment 53214 [details])
> +<script src="resources/utilities.js"></script>
> 
> I don't think we can edit these files because they are generated from the
> make-test-wrappers script.

I added the external JavaScript file to the TEMPLATE.html file. So, when you run make-script-test-wrappers, it will generate ipv4.html, trivial.html, and query.html with this external JavaScript.

> 
> +  ["as#df", "as%23df"],
> 
> I don't think this result is correct, but I agree that we should just literally
> land what's in the unit tests.  We can do a separate pass to understand the
> different behaviors here.

Yes, I was skeptical about this as well being valid. I can remove it.
Comment 4 Adam Barth 2010-04-12 20:35:43 PDT
Comment on attachment 53214 [details]
Layout Test

I stand corrected.  You are awesome.  Let's leave the test in.  I'd like to do a pass through the whole test suite once we've got it checked in.
Comment 5 WebKit Commit Bot 2010-04-12 21:53:06 PDT
Comment on attachment 53214 [details]
Layout Test

Clearing flags on attachment: 53214

Committed r57501: <http://trac.webkit.org/changeset/57501>
Comment 6 WebKit Commit Bot 2010-04-12 21:53:12 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Jonathan Dixon 2010-04-12 23:37:48 PDT
Tip for the follow up pass: you can use http://rishida.net/tools/conversion/ to do the character set conversion utf 8 -> utf 16, e.g.
q=\xe4\xbd\xa0\xe5\xa5\xbd translates to q=\u4F60\u597D in JS escaping.
(i.e. enter "e4 bd a0 e5 a5 bd" into the UTF-8 code units box)
Comment 8 Adam Barth 2010-04-13 00:40:45 PDT
Sweet!