Bug 196710 – Web Inspector: Audit: there should be a default test for WebInspectorAudit.Resources functionality

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

[Devin Rousso]

This will likely require that we expose another way for values to be returned, possibly even for arbitrary JSON objects (e.g. there's no non-error way of sending back a resource's contents).

[Radar WebKit Bug Importer]

<rdar://problem/49712348>

[Devin Rousso]

Created attachment 370671 [details]
Patch

My local build is currently not building (unrelated), so this is a test using the bots instead :|

[EWS Watchlist]

This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.

[EWS Watchlist]

Comment on attachment 370671 [details]
Patch

Attachment 370671 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12298572

New failing tests:
inspector/audit/data-errors.html
inspector/audit/data-domAttributes.html
inspector/audit/basic-promise.html
inspector/audit/data-domNodes.html
inspector/model/auditTestCaseResult.html
inspector/audit/manager-start-setup.html
inspector/audit/basic-async.html
inspector/audit/basic-object.html
inspector/audit/basic-error.html

[EWS Watchlist]

Created attachment 370673 [details]
Archive of layout-test-results from ews103 for mac-highsierra

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

[Devin Rousso]

Created attachment 370674 [details]
Patch

Fix typo :P

[EWS Watchlist]

Comment on attachment 370674 [details]
Patch

Attachment 370674 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12299112

New failing tests:
inspector/audit/data-errors.html
inspector/audit/data-domAttributes.html
inspector/audit/basic-promise.html
inspector/audit/data-domNodes.html
inspector/model/auditTestCaseResult.html
inspector/audit/manager-start-setup.html
inspector/audit/basic-async.html
inspector/audit/basic-object.html
inspector/audit/basic-error.html

[EWS Watchlist]

Created attachment 370679 [details]
Archive of layout-test-results from ews103 for mac-highsierra

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

[EWS Watchlist]

Comment on attachment 370674 [details]
Patch

Attachment 370674 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12299133

New failing tests:
inspector/audit/data-errors.html
inspector/audit/data-domAttributes.html
inspector/audit/basic-promise.html
inspector/audit/data-domNodes.html
inspector/model/auditTestCaseResult.html
inspector/audit/manager-start-setup.html
inspector/audit/basic-async.html
inspector/audit/basic-object.html
inspector/audit/basic-error.html

[EWS Watchlist]

Created attachment 370680 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

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

[EWS Watchlist]

Comment on attachment 370674 [details]
Patch

Attachment 370674 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12299240

New failing tests:
inspector/audit/data-errors.html
inspector/audit/data-domAttributes.html
inspector/audit/basic-promise.html
inspector/audit/data-domNodes.html
inspector/model/auditTestCaseResult.html
inspector/audit/manager-start-setup.html
inspector/audit/basic-async.html
inspector/audit/basic-object.html
inspector/audit/basic-error.html

[EWS Watchlist]

Created attachment 370683 [details]
Archive of layout-test-results from ews114 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 370674 [details]
Patch

Attachment 370674 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12299790

New failing tests:
fast/shadow-dom/svg-use-href-change-in-shadow-tree.html
css3/filters/blur-various-radii.html

[EWS Watchlist]

Created attachment 370688 [details]
Archive of layout-test-results from ews211 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews211  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Devin Rousso]

Created attachment 370699 [details]
Patch

[EWS Watchlist]

Comment on attachment 370699 [details]
Patch

Attachment 370699 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12302608

New failing tests:
inspector/model/auditTestCaseResult.html

[EWS Watchlist]

Created attachment 370702 [details]
Archive of layout-test-results from ews101 for mac-highsierra

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

[EWS Watchlist]

Comment on attachment 370699 [details]
Patch

Attachment 370699 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12302625

New failing tests:
inspector/model/auditTestCaseResult.html

[EWS Watchlist]

Created attachment 370703 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

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

[EWS Watchlist]

Comment on attachment 370699 [details]
Patch

Attachment 370699 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12302656

New failing tests:
inspector/model/auditTestCaseResult.html

[EWS Watchlist]

Created attachment 370704 [details]
Archive of layout-test-results from ews113 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Devin Rousso]

Created attachment 370707 [details]
Patch

[Devin Rousso]

Comment on attachment 370707 [details]
Patch

There should be a provided `data-custom` default Audit that can act as a demo of both this special functionality and a good way to see the UI for custom return values.

[Devin Rousso]

Created attachment 370948 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 370948 [details]
Patch

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

rs=me

> Source/WebInspectorUI/UserInterface/Models/AuditTestCase.js:285
> +                        } catch (e) {

Style: No need for the `(e)` if it is not used.

> Source/WebInspectorUI/UserInterface/Models/AuditTestCase.js:287
> +                            if (key !== "__proto__")

Can we just skip __proto__ earlier, I don't think we'd want to set it.

> Source/WebInspectorUI/UserInterface/Models/AuditTestCaseResult.js:136
> +                if (key === "domNodes" || key === "domAttributes" || key === "errors") {

Might be useful to make a static function for special attributes?

    AuditTestCaseResult.isSpecialAttribute(key)

> Source/WebInspectorUI/UserInterface/Views/AuditTestCaseContentView.js:36
> +        this._resultDataNonSpecialContainer = null;

Instead of "NonSpecial" maybe "General" or "CustomData" would be a better name.

> Source/WebInspectorUI/UserInterface/Views/AuditTestCaseContentView.js:141
> +            let nonSpecialData = Object.filter(resultData, (key) => key !== "domNodes" && key !== "errors");

domAttributes?

> LayoutTests/inspector/unit-tests/object-utilities-expected.txt:26
> +PASS: filter should remove all entries where the key isn't in ["a","b","c"]

Nit: These normally end in a period.

[Devin Rousso]

Comment on attachment 370948 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Models/AuditTestCaseResult.js:136
>> +                if (key === "domNodes" || key === "domAttributes" || key === "errors") {
> 
> Might be useful to make a static function for special attributes?
> 
>     AuditTestCaseResult.isSpecialAttribute(key)

It's less "special", more that we require that the value is an array.

>> Source/WebInspectorUI/UserInterface/Views/AuditTestCaseContentView.js:141
>> +            let nonSpecialData = Object.filter(resultData, (key) => key !== "domNodes" && key !== "errors");
> 
> domAttributes?

I'm fine showing `domAttributes` as part of `this._resultDataGeneralContainer`, as it otherwise would only be visible if `domNodes` has values in it.

[Devin Rousso]

Created attachment 370989 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 370989 [details]
Patch

Clearing flags on attachment: 370989

Committed r245914: <https://trac.webkit.org/changeset/245914>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.