Bug 147787 – Web Inspector: use different namespaces in test fixtures for protocol tests and frontend tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Blaze Burg]

IOW, rename InspectorTest to ProtocolTest or something similar in protocol-test.js.

[Blaze Burg]

Current refactoring:

* Protocol-related things in ProtocolTestStub get renamed to InspectorProtocol
* Test things (log, assert, etc) get renamed to ProtocolTest
* Everything else is left alone

In the next refactor, Test things will be refactored to be shared between Protocol and Frontend tests. Until then, the two implementations have different names.

[Blaze Burg]

Created attachment 258664 [details]
Proposed Fix

[Build Bot]

Comment on attachment 258664 [details]
Proposed Fix

Attachment 258664 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/40821

New failing tests:
http/tests/inspector/console/access-inspected-object.html
http/tests/inspector/page/loading-iframe-document-node.html

[Build Bot]

Created attachment 258667 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 258664 [details]
Proposed Fix

Attachment 258664 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/40834

New failing tests:
http/tests/inspector/console/access-inspected-object.html
http/tests/inspector/page/loading-iframe-document-node.html

[Build Bot]

Created attachment 258669 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Devin Rousso]

Comment on attachment 258664 [details]
Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=258664&action=review

> LayoutTests/http/tests/inspector/resources/ProtocolTestStub.js:113
> +InspectorProtocol.checkForError = function(responseObject)

Will this also work for functions that return promises?  Also, I'm not sure about whether this is true or not, but are there any cases where a callback function/promise won't have a responseObject?  If so, I think you should check to see that the response object exists (and if it doesn't then throw the same error).  If that isn't true, you can ignore me.

That's really the only comment I have.

[Blaze Burg]

Created attachment 258673 [details]
Proposed Fix

I missed the few http/tests/inspector/ tests.

[Blaze Burg]

Comment on attachment 258664 [details]
Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=258664&action=review

>> LayoutTests/http/tests/inspector/resources/ProtocolTestStub.js:113
>> +InspectorProtocol.checkForError = function(responseObject)
> 
> Will this also work for functions that return promises?  Also, I'm not sure about whether this is true or not, but are there any cases where a callback function/promise won't have a responseObject?  If so, I think you should check to see that the response object exists (and if it doesn't then throw the same error).  If that isn't true, you can ignore me.
> 
> That's really the only comment I have.

This is reordered code. I want to delete it eventually (the protocol layer should handle this, not test code), but not in this patch.

[WebKit Commit Bot]

Comment on attachment 258673 [details]
Proposed Fix

Clearing flags on attachment: 258673

Committed r188267: <http://trac.webkit.org/changeset/188267>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.