RESOLVED FIXED 80650
[WebSocket] Add a layout test where server force to disable all extensions
https://bugs.webkit.org/show_bug.cgi?id=80650
Summary [WebSocket] Add a layout test where server force to disable all extensions
Takashi Toyoshima
Reported 2012-03-08 16:00:30 PST
Currently, WebSocket extensions which WebKit implement are all enabled by default. But this enabled extension list could be changed when WebKit support other extensions or rename existing extensions. It means that the expectation of a test which verify extensions property depends on WebKit. This is not good for embedder's unit tests. So, I'd like to add a layout test where server pyewebsocket just omit all extensions and the extensions property returns empty string.
Attachments
Patch (7.10 KB, patch)
2012-03-08 16:33 PST, Takashi Toyoshima
no flags
Patch (7.08 KB, patch)
2012-03-08 17:14 PST, Takashi Toyoshima
no flags
Patch (7.94 KB, patch)
2012-03-08 21:26 PST, Takashi Toyoshima
no flags
Takashi Toyoshima
Comment 1 2012-03-08 16:33:35 PST
Kenichi Ishibashi
Comment 2 2012-03-08 17:07:26 PST
Comment on attachment 130937 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130937&action=review > LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension.html:25 > + shouldBeEqualToString("ws.extensions", "x-webkit-deflate-frame"); shouldBeEqualToString("ws.extensions", ""); ?
Takashi Toyoshima
Comment 3 2012-03-08 17:12:12 PST
Comment on attachment 130937 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130937&action=review >> LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension.html:25 >> + shouldBeEqualToString("ws.extensions", "x-webkit-deflate-frame"); > > shouldBeEqualToString("ws.extensions", ""); ? Oops, right. I mistakenly overwrite the expectation.
Takashi Toyoshima
Comment 4 2012-03-08 17:14:51 PST
Kent Tamura
Comment 5 2012-03-08 20:34:36 PST
Comment on attachment 130940 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130940&action=review > LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension-expected.txt:7 > +PASS ws.extensions is "" > +PASS ws.extensions is "" > +PASS ws.extensions is "" It's hard to understand what are tested here with this result. At least we should show: Check the value before opening the connection: PASS ws.extensions is "" PASS ws.extensions = "foo"; ws.extensions is "" Check the value after opening the connection: PASS ws.extensions is "" > LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension.html:21 > +ws.extensions = "foo"; > +shouldBeEqualToString("ws.extensions", ""); I prefer shouldBeEqualToString("ws.extensions = 'foo'; ws.extensions", ""); > LayoutTests/http/tests/websocket/tests/hybi/extensions-expected.txt:7 > PASS ws.extensions is "" > PASS ws.extensions is "" > +PASS ws.extensions is "x-webkit-deflate-frame" I'd like to make the same comment with echo-with-no-extension-expected.txt.
Takashi Toyoshima
Comment 6 2012-03-08 21:25:20 PST
Comment on attachment 130940 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130940&action=review >> LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension-expected.txt:7 >> +PASS ws.extensions is "" > > It's hard to understand what are tested here with this result. At least we should show: > > Check the value before opening the connection: > PASS ws.extensions is "" > PASS ws.extensions = "foo"; ws.extensions is "" > Check the value after opening the connection: > PASS ws.extensions is "" Agreed. >> LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension.html:21 >> +shouldBeEqualToString("ws.extensions", ""); > > I prefer shouldBeEqualToString("ws.extensions = 'foo'; ws.extensions", ""); I see. >> LayoutTests/http/tests/websocket/tests/hybi/extensions-expected.txt:7 >> +PASS ws.extensions is "x-webkit-deflate-frame" > > I'd like to make the same comment with echo-with-no-extension-expected.txt. Done.
Takashi Toyoshima
Comment 7 2012-03-08 21:26:01 PST
Kent Tamura
Comment 8 2012-03-08 21:27:41 PST
Comment on attachment 130975 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=130975&action=review > LayoutTests/http/tests/websocket/tests/hybi/echo-with-no-extension.html:21 > +// extensions attribute is read-only. > +debug("Check if the value is read only:"); nit: The comment is not helpful. > LayoutTests/http/tests/websocket/tests/hybi/extensions.html:20 > // extensions attribute is read-only. ditto.
WebKit Review Bot
Comment 9 2012-03-09 00:48:34 PST
Comment on attachment 130975 [details] Patch Clearing flags on attachment: 130975 Committed r110274: <http://trac.webkit.org/changeset/110274>
WebKit Review Bot
Comment 10 2012-03-09 00:48:40 PST
All reviewed patches have been landed. Closing bug.
Nandor Huszka
Comment 11 2012-03-09 01:51:00 PST
This revision (r110274) causes fail on Qt. The diff: --- /ramdisk/qt-linux-64-release/build/layout-test-results/http/tests/websocket/tests/hybi/extensions-expected.txt +++ /ramdisk/qt-linux-64-release/build/layout-test-results/http/tests/websocket/tests/hybi/extensions-actual.txt @@ -7,7 +7,7 @@ Check if the value is read only: PASS ws.extensions = 'foo'; ws.extensions is "" Check the value after the connection is established: -PASS ws.extensions is "x-webkit-deflate-frame" +FAIL ws.extensions should be x-webkit-deflate-frame. Was . PASS successfullyParsed is true TEST COMPLETE
Kent Tamura
Comment 12 2012-03-09 02:01:53 PST
(In reply to comment #11) > This revision (r110274) causes fail on Qt. The diff: It's the same reason with compressed-control-frame.html, deflate-frame-invalid-parameter.html, deflate-frame-parameter.html, and handshake-fail-by-extensions-header.html. They are skipped in Qt.
Nandor Huszka
Comment 13 2012-03-09 03:40:41 PST
(In reply to comment #12) > It's the same reason with compressed-control-frame.html, deflate-frame-invalid-parameter.html, deflate-frame-parameter.html, and handshake-fail-by-extensions-header.html. They are skipped in Qt. Thank you for the information, test will be inserted to that group.
Note You need to log in before you can comment on or make changes to this bug.