Bug 27492 – Web Sockets Test Infrastructure Part 3/3: Example test

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Yuzo Fujishima]

Add test infrastructure for Web Sockets (http://www.w3.org/TR/websockets/).

This bug is Part 3/3: Example test.

Other parts are:
Part 1/3: Server
Part 2/3: Patch to run-webkit-tests
Parts must be submitted in the order Part1, Part2, then Part3.

Perhaps Part 3 doesn't need to be submitted.
I create it nonetheless, hoping it will help understanding Part 1 and Part 2.

[Yuzo Fujishima]

Links:

Part 1/3: Server: https://bugs.webkit.org/show_bug.cgi?id=27490
Part 2/3: Patch to run-webkit-tests: https://bugs.webkit.org/show_bug.cgi?id=27491
Part 3/3: Example test: https://bugs.webkit.org/show_bug.cgi?id=27492

[Yuzo Fujishima]

Created attachment 33162 [details]
Patch to add example Web Sockets test.

[Yuzo Fujishima]

Created attachment 34056 [details]
Example Web Socket Tests, same-origin and cross-origin.

New example tests for the new test server.

[Eric Seidel (no email)]

Comment on attachment 34056 [details]
Example Web Socket Tests, same-origin and cross-origin.

You forgot to mark this as a patch.  If you use tools like "bugzilla-tool" it will do all that for you.  "bugzilla-tool post-diff" or "bugzilla-tool post-commits"

[Eric Seidel (no email)]

Comment on attachment 34056 [details]
Example Web Socket Tests, same-origin and cross-origin.

Great that these are js tests!  Bad that you had to copy the files.  We really should  fix bug 25915 instead.

ChangeLog is duplicated.

 13     debug('<span class="pass">TEST COMPLETE</span>');
is instead a testPassed() call.

Style:
 if (window.layoutTestController) {
 4     window.layoutTestController.waitUntilDone();
 5 }

description() is the right call here:
debug("NOOP Web Socket test");

r- for various nits above.

[Yuzo Fujishima]

Created attachment 41933 [details]
Add a simple web socket test.

[Yuzo Fujishima]

Hi, reviewers and committers,

I've updated the example test to suit the latest test framework.

This patch now requires that:
- https://bugs.webkit.org/show_bug.cgi?id=27491 must be committed.
- Web Socket must be functional in the test shell.

So please hold committing this until the above conditions are met,
even if this patch looks good. :)

Yuzo

[Eric Seidel (no email)]

Comment on attachment 41933 [details]
Add a simple web socket test.

Needs a ChagneLog.

[Yuzo Fujishima]

Created attachment 41998 [details]
Add a simple web socket test.

[Yuzo Fujishima]

Sorry for the missing ChangeLog.

Can you take a look again?

Note: This patch requires that:
- https://bugs.webkit.org/show_bug.cgi?id=27491 must be committed before this.
- Web Socket must be functional in the test shell.

[David Levin]

Comment on attachment 41998 [details]
Add a simple web socket test.

Just some minor issues to address and this should be fine to go in.

> diff --git a/LayoutTests/websocket/tests/resources/simple.js b/LayoutTests/websocket/tests/resources/simple.js

I think this is in the wrong place after Hamaji's changes.


> diff --git a/LayoutTests/websocket/tests/simple_wsh.py b/LayoutTests/websocket/tests/simple_wsh.py
> new file mode 100755
> index 0000000..5dc71bd
> --- /dev/null
> +++ b/LayoutTests/websocket/tests/simple_wsh.py
> @@ -0,0 +1,42 @@
> +# Copyright 2009, Google Inc.
> +# All rights reserved.

Please use the standard copyright style for WebKit.

Ideally the copyright would be of this format:
   Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
   Copyright (C) 2009 Somebody <email>


> +def web_socket_do_extra_handshake(request):
> +    pass  # Always accept.

In keeping with out Webkit code, please use one space before end of line comments.


> +# vi:sts=4 sw=4 et

No other WebKit sources have this vi formating info. Please remove it.

[Yuzo Fujishima]

Created attachment 42452 [details]
Add a simple web socket test.

[Yuzo Fujishima]

Thank you for the review.

Can you take another look?

Please commit this after Web Socket is enabled for the test shell.
I'll ping you (or ask someone to commit) when ready.

Yuzo

[David Levin]

Comment on attachment 42452 [details]
Add a simple web socket test.

> Please commit this after Web Socket is enabled for the test shell.

Ideally you'd commit this as soon as possible. The tests could always be disabled temporarily for chromium if it isn't ready.

[Yuzo Fujishima]

Created attachment 42927 [details]
Add a simple web socket test.

[Yuzo Fujishima]

Hi, David,

I've added an entry to each of LayoutTests/platform/*/Skipped files.
Can you review and submit this patch if it looks good?

Yuzo

[Yuzo Fujishima]

Created attachment 42933 [details]
Add a simple web socket test.

[Yuzo Fujishima]

Created attachment 42934 [details]
Add a simple web socket test.

[Yuzo Fujishima]

Created attachment 42935 [details]
Add a simple web socket test.

[Yuzo Fujishima]

I've made minor corrections:
- single quote to double quote for a string literal
- added missing semicolons to end of line

Yuzo

[WebKit Commit Bot]

Comment on attachment 42935 [details]
Add a simple web socket test.

Clearing flags on attachment: 42935

Committed r50794: <http://trac.webkit.org/changeset/50794>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.