Bug 24569 – Add some cookies tests

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Julien Chaffraix]

As suggested by Kevin, adding some cookies tests would be a good start in solving bug 14730. This will also be a nice addition to our tests.

As there is no such test suite to my knowledge, we will create the framework from scratch. My idea is to use several XHRs to get/set/clear the cookies by using custom HTTP headers rewritten at the server side.

First version forthcoming.

[Julien Chaffraix]

Created attachment 28573 [details]
First try: includes some basic testing

This is a very first version, it does basic testing for the moment and will be extended to integrate real world cookies. I am interested in comments in the design taken.

[David Kilzer (:ddkilzer)]

Comment on attachment 28573 [details]
First try: includes some basic testing

This is really good stuff!!  r=me with a few nits:

>Index: LayoutTests/http/tests/cookies/resources/cookies-test-pre.js
>+function shouldBe(_a, _b)
>+function shouldBeEqualToString(a, b)
>+function shouldBeUndefined(_a)
>+function shouldThrow(_a, _e)

These functions should be indented by 4 spaces initially instead of just 2 spaces.

[David Kilzer (:ddkilzer)]

Hmm...http/tests/cookies/simple-cookies.html fails when run on a local Debug build of ToT WebKit r43601 on Leopard 10.5.7.

[David Kilzer (:ddkilzer)]

(In reply to comment #3)
> Hmm...http/tests/cookies/simple-cookies.html fails when run on a local Debug
> build of ToT WebKit r43601 on Leopard 10.5.7.

Oops!  It helps if you make getCookie.cgi and setCookie.cgi executable.  :)

[David Kilzer (:ddkilzer)]

(In reply to comment #4)
> (In reply to comment #3)
> > Hmm...http/tests/cookies/simple-cookies.html fails when run on a local Debug
> > build of ToT WebKit r43601 on Leopard 10.5.7.
> 
> Oops!  It helps if you make getCookie.cgi and setCookie.cgi executable.  :)

The "setting a cookie that timed out" check appears to be incorrect.  It fails in both Firefox 3.0.8 and Safari 4 Public Beta on Leopard 10.5.7.

[...]
PASS cookie is test=foobar
Check setting a cookie that timed out
FAIL cookie was test=foobar. Expected 
[...]

Otherwise the simple-cookies.html test looks good.

[David Kilzer (:ddkilzer)]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Hmm...http/tests/cookies/simple-cookies.html fails when run on a local Debug
> > > build of ToT WebKit r43601 on Leopard 10.5.7.
> > 
> > Oops!  It helps if you make getCookie.cgi and setCookie.cgi executable.  :)
> 
> The "setting a cookie that timed out" check appears to be incorrect.  It fails
> in both Firefox 3.0.8 and Safari 4 Public Beta on Leopard 10.5.7.
> 
> [...]
> PASS cookie is test=foobar
> Check setting a cookie that timed out
> FAIL cookie was test=foobar. Expected 
> [...]

*sigh*  Had to make clearCookies.cgi executable as well.

I think clearCookies.cgi should do case-insensitive matches and use the original Expires data format:

-if ($cookie =~ /Max-Age/) {
-    $cookie =~ s/Max-Age *= *[0-9]+/Max-Age=0/;
+if ($cookie =~ /Max-Age/i) {
+    $cookie =~ s/Max-Age *= *[0-9]+/Max-Age=0/i;
     $handled = 1;
 } 
-if ($cookie =~ /Expires/) {
+if ($cookie =~ /Expires/i) {
     # Set the "Expires" field to UNIX epoch
-    $cookie =~ s/Expires *= *[^;]+/Expires=Thu Jan 01 1970 00:00:00 GMT/;
+    $cookie =~ s/Expires *= *[^;]+/Expires=Thu, 01 Jan 1970 00:00:00 GMT/i;
     $handled = 1;
 } 

I'll land this on Monday if there are no other objections.

[David Kilzer (:ddkilzer)]

And we need to skip the cookies-test-(post|pre).js files when running the make-js-test-wrappers script:

     for my $file (@files) {
         next if $file =~ /js-test-.*\.js$/;
+        next if $file =~ /cookies-test-(post|pre)\.js$/;
         next if $file =~ /standalone-.*\.js$/;
         next if $file =~ /SVGTestCase\.js/;

[David Kilzer (:ddkilzer)]

Created attachment 30448 [details]
Minor changes after testing on Tiger

This patch applies on top of the original (plus the changes in Comment# 6 and Comment #7).

After testing the patch on my Tiger system at home, I realized that Tiger can't clear cookies by setting Max-Age=0, so I made the following changes:

- Split simple-cookies.html into simple-cookies-max-age.html and multiple-cookies.html.
- Added http/tests/cookies/simple-cookies-max-age.html to the platform/mac-tiger/Skipped list.
- Created simple-cookies-expired.html based on simple-cookies-max-age.html.
- Changed clearCookies.cgi to set both Max-Age=0 and Expires=<UNIX epoch start date> when expiring a cookie.
- Added a clearAllCookies() method to cookies-test-pre.js and added it to the top of each cookie test.
- Fixed a warning in getCookies.cgi.

[David Kilzer (:ddkilzer)]

$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/http/tests/cookies/multiple-cookies-expected.txt
	A	LayoutTests/http/tests/cookies/multiple-cookies.html
	A	LayoutTests/http/tests/cookies/resources/TEMPLATE.html
	A	LayoutTests/http/tests/cookies/resources/clearCookies.cgi
	A	LayoutTests/http/tests/cookies/resources/cookies-test-post.js
	A	LayoutTests/http/tests/cookies/resources/cookies-test-pre.js
	A	LayoutTests/http/tests/cookies/resources/cookies-test-style.css
	A	LayoutTests/http/tests/cookies/resources/getCookies.cgi
	A	LayoutTests/http/tests/cookies/resources/multiple-cookies.js
	A	LayoutTests/http/tests/cookies/resources/setCookies.cgi
	A	LayoutTests/http/tests/cookies/resources/simple-cookies-expired.js
	A	LayoutTests/http/tests/cookies/resources/simple-cookies-max-age.js
	A	LayoutTests/http/tests/cookies/simple-cookies-expired-expected.txt
	A	LayoutTests/http/tests/cookies/simple-cookies-expired.html
	A	LayoutTests/http/tests/cookies/simple-cookies-max-age-expected.txt
	A	LayoutTests/http/tests/cookies/simple-cookies-max-age.html
	M	LayoutTests/platform/mac-tiger/Skipped
	M	WebKitTools/ChangeLog
	M	WebKitTools/Scripts/make-js-test-wrappers
Committed r43851

http://trac.webkit.org/changeset/43851

[David Kilzer (:ddkilzer)]

Had to add http/tests/cookies/simple-cookies-expired.html to the Skipped list file for Windows (see Bug 25861):

$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/platform/win/Skipped
Committed r43853

http://trac.webkit.org/changeset/43853

[Eric Seidel (no email)]

http/tests/cookies/simple-cookies-expired.html is flaky.