Bug 196710 - Web Inspector: Audit: there should be a default test for WebInspectorAudit.Resources functionality
Summary: Web Inspector: Audit: there should be a default test for WebInspectorAudit.Re...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on: WebInspectorAuditTab
Blocks:
  Show dependency treegraph
 
Reported: 2019-04-08 14:46 PDT by Devin Rousso
Modified: 2019-05-30 17:12 PDT (History)
11 users (show)

See Also:


Attachments
Patch (31.89 KB, patch)
2019-05-26 22:57 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-highsierra (3.33 MB, application/zip)
2019-05-26 23:46 PDT, Build Bot
no flags Details
Patch (34.47 KB, patch)
2019-05-27 00:26 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-highsierra (3.16 MB, application/zip)
2019-05-27 01:32 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (2.79 MB, application/zip)
2019-05-27 01:46 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-highsierra (3.03 MB, application/zip)
2019-05-27 02:23 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews211 for win-future (14.01 MB, application/zip)
2019-05-27 04:45 PDT, Build Bot
no flags Details
Patch (44.71 KB, patch)
2019-05-27 11:39 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-highsierra (3.08 MB, application/zip)
2019-05-27 12:45 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-highsierra-wk2 (2.71 MB, application/zip)
2019-05-27 12:58 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-highsierra (2.89 MB, application/zip)
2019-05-27 13:27 PDT, Build Bot
no flags Details
Patch (48.40 KB, patch)
2019-05-27 14:06 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (56.67 KB, patch)
2019-05-30 09:41 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (56.59 KB, patch)
2019-05-30 16:06 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2019-04-08 14:46:58 PDT
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).
Comment 1 Radar WebKit Bug Importer 2019-04-08 14:48:33 PDT
<rdar://problem/49712348>
Comment 2 Devin Rousso 2019-05-26 22:57:12 PDT
Created attachment 370671 [details]
Patch

My local build is currently not building (unrelated), so this is a test using the bots instead :|
Comment 3 Build Bot 2019-05-26 22:59:33 PDT Comment hidden (obsolete)
Comment 4 Build Bot 2019-05-26 23:46:05 PDT Comment hidden (obsolete)
Comment 5 Build Bot 2019-05-26 23:46:07 PDT Comment hidden (obsolete)
Comment 6 Devin Rousso 2019-05-27 00:26:35 PDT
Created attachment 370674 [details]
Patch

Fix typo :P
Comment 7 Build Bot 2019-05-27 01:32:26 PDT Comment hidden (obsolete)
Comment 8 Build Bot 2019-05-27 01:32:28 PDT Comment hidden (obsolete)
Comment 9 Build Bot 2019-05-27 01:46:24 PDT Comment hidden (obsolete)
Comment 10 Build Bot 2019-05-27 01:46:26 PDT Comment hidden (obsolete)
Comment 11 Build Bot 2019-05-27 02:23:16 PDT Comment hidden (obsolete)
Comment 12 Build Bot 2019-05-27 02:23:18 PDT Comment hidden (obsolete)
Comment 13 Build Bot 2019-05-27 04:45:46 PDT Comment hidden (obsolete)
Comment 14 Build Bot 2019-05-27 04:45:50 PDT Comment hidden (obsolete)
Comment 15 Devin Rousso 2019-05-27 11:39:19 PDT
Created attachment 370699 [details]
Patch
Comment 16 Build Bot 2019-05-27 12:45:06 PDT Comment hidden (obsolete)
Comment 17 Build Bot 2019-05-27 12:45:08 PDT Comment hidden (obsolete)
Comment 18 Build Bot 2019-05-27 12:58:16 PDT Comment hidden (obsolete)
Comment 19 Build Bot 2019-05-27 12:58:18 PDT Comment hidden (obsolete)
Comment 20 Build Bot 2019-05-27 13:27:02 PDT Comment hidden (obsolete)
Comment 21 Build Bot 2019-05-27 13:27:04 PDT Comment hidden (obsolete)
Comment 22 Devin Rousso 2019-05-27 14:06:40 PDT
Created attachment 370707 [details]
Patch
Comment 23 Devin Rousso 2019-05-29 23:20:38 PDT
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.
Comment 24 Devin Rousso 2019-05-30 09:41:52 PDT
Created attachment 370948 [details]
Patch
Comment 25 Joseph Pecoraro 2019-05-30 15:24:25 PDT
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.
Comment 26 Devin Rousso 2019-05-30 16:03:40 PDT
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.
Comment 27 Devin Rousso 2019-05-30 16:06:44 PDT
Created attachment 370989 [details]
Patch
Comment 28 WebKit Commit Bot 2019-05-30 17:12:09 PDT
Comment on attachment 370989 [details]
Patch

Clearing flags on attachment: 370989

Committed r245914: <https://trac.webkit.org/changeset/245914>
Comment 29 WebKit Commit Bot 2019-05-30 17:12:11 PDT
All reviewed patches have been landed.  Closing bug.