| Summary: | In testing handshake-error case, web_socket_do_extra_handshake calls non-existent method | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yuzo Fujishima <yuzo> | ||||||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | ap, eric | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Attachments: |
|
||||||||||
Created attachment 43148 [details]
Fix: web_socket_do_extra_handshake calls non-existent method
Hi, reviewers, Can you review this and submit it if it looks good? Sorry for not catching the bug when proposing the original patch. Yuzo Created attachment 43149 [details]
Fix: web_socket_do_extra_handshake calls non-existent method
Comment on attachment 43149 [details]
Fix: web_socket_do_extra_handshake calls non-existent method
Why doesn't this mistake affect test output? Should it?
r=me
Comment on attachment 43149 [details]
Fix: web_socket_do_extra_handshake calls non-existent method
In response to your above comment requesting submission: You can mark your patches commit-queue=? if you want them automatically submitted.
See http://webkit.org/coding/contributing.html for documentation of the review and commit-queue flags. Comment on attachment 43149 [details]
Fix: web_socket_do_extra_handshake calls non-existent method
Rejecting patch 43149 from commit-queue.
Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 11623 test cases.
http/tests/xmlhttprequest/access-control-basic-allow-access-control-origin-header.html -> crashed
Exiting early after 1 failures. 9152 tests run.
344.54s total testing time
9151 test cases (99%) succeeded
1 test case (<1%) crashed
5 test cases (<1%) had stderr output
Comment on attachment 43149 [details] Fix: web_socket_do_extra_handshake calls non-existent method False rejection caused by bug 31461. Comment on attachment 43149 [details]
Fix: web_socket_do_extra_handshake calls non-existent method
Rejecting patch 43149 from commit-queue.
Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 11623 test cases.
inspector/console-dir.html -> crashed
Exiting early after 1 failures. 9314 tests run.
368.05s total testing time
9313 test cases (99%) succeeded
1 test case (<1%) crashed
5 test cases (<1%) had stderr output
Comment on attachment 43149 [details] Fix: web_socket_do_extra_handshake calls non-existent method False rejection due to bug 31461. Comment on attachment 43149 [details]
Fix: web_socket_do_extra_handshake calls non-existent method
Rejecting patch 43149 from commit-queue.
Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 11623 test cases.
http/tests/xmlhttprequest/abort-crash.html -> crashed
Exiting early after 1 failures. 9147 tests run.
341.11s total testing time
9146 test cases (99%) succeeded
1 test case (<1%) crashed
5 test cases (<1%) had stderr output
Comment on attachment 43149 [details] Fix: web_socket_do_extra_handshake calls non-existent method I'm disabling the commit-queue until bug 31461 can be solved. Sorry. I expect we'll find a solution to bug 31461 on monday at which point I"ll restart the queue and this will be committed. Comment on attachment 43149 [details]
Fix: web_socket_do_extra_handshake calls non-existent method
Rejecting patch 43149 from commit-queue.
Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 11631 test cases.
websocket/tests/handshake-error.html -> failed
Exiting early after 1 failures. 11630 tests run.
387.22s total testing time
11629 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
6 test cases (<1%) had stderr output
Sorry this error wasn't discovered sooner. The commit-queue stops running tests after the first failure, and bug 31461 was masking this failure before. Created attachment 43482 [details]
Fix: web_socket_do_extra_handshake calls non-existent method
Sorry for the test breakage. I thought this test was in Skipped. I believe this patch itself is correct and should be checked in: it just revealed a hidden bug. I've filed https://bugs.webkit.org/show_bug.cgi?id=31659 I added (back) this test to Skipped. Yuzo Comment on attachment 43482 [details]
Fix: web_socket_do_extra_handshake calls non-existent method
OK.
Comment on attachment 43482 [details]
Fix: web_socket_do_extra_handshake calls non-existent method
Rejecting patch 43482 from commit-queue.
Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Eric Seidel', '--force']" exit_code: 1
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/mac/Skipped
Hunk #1 FAILED at 101.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/mac/Skipped.rej
patching file LayoutTests/websocket/tests/handshake-error_wsh.py
Manually committed as <http://trac.webkit.org/changeset/51508>. I re-wrote the ChangeLog to make it a bit more informative. http://trac.webkit.org/changeset/51509 I don't think the tools did anything wrong here! :) I reviewed a later version of the patch. I'm not seeking credit, just making sure you're not unfairly blaming our innocent tools! ;) Btw, if you're using bugzilla-tool for any of this, I believe both land-* and apply-* accept a --reviewer argument which allows you to override reviewers in cases like this where the bug might have a different reviewer than you want the final patch to have. :) > I don't think the tools did anything wrong here! :) I reviewed a later version
> of the patch. I'm not seeking credit, just making sure you're not unfairly
> blaming our innocent tools! ;)
OK, I didn't know you actually reviewed the change, not just set the flag per earlier review. The latter has certainly happened in the past (many of use did it, including myself).
The reason I see tools share part of the blame is that almost good patches need to be reworked more often now, so reviews get forwarded by whoever gets to it first. This is mostly a good thing, but it makes reviewer info wrong in Bugzilla and in ChangeLogs.
|
In testing handshake-error case, web_socket_do_extra_handshake calls non-existent method and the following error is raised: WARNING:root:mod_pywebsocket: web_socket_do_extra_handshake raised exception: Traceback (most recent call last): File "/Users/yuzo/src/chromium/src/third_party/pywebsocket/mod_pywebsocket/dispatch.py", line 163, in do_extra_handshake do_extra_handshake_(request) File "<string>", line 34, in web_socket_do_extra_handshake AttributeError: '_StandaloneRequest' object has no attribute 'write'