Bug 190853 – Web Inspector: Audit: show metadata for results

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

[Devin Rousso]

This would include:
 - date run
 - time taken
 - website
 - system info (maybe)

[Radar WebKit Bug Importer]

<rdar://problem/45527623>

[Devin Rousso]

Created attachment 353466 [details]
Patch

[Devin Rousso]

Created attachment 353467 [details]
[Image] After Patch is applied

[EWS Watchlist]

Comment on attachment 353466 [details]
Patch

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

New failing tests:
inspector/model/auditTestCaseResult.html
inspector/model/auditTestGroupResult.html

[EWS Watchlist]

Created attachment 353468 [details]
Archive of layout-test-results from ews101 for mac-sierra

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

[EWS Watchlist]

Comment on attachment 353466 [details]
Patch

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

New failing tests:
inspector/model/auditTestGroupResult.html
inspector/model/auditTestCaseResult.html

[EWS Watchlist]

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

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

[Devin Rousso]

Created attachment 353471 [details]
Patch

[Blaze Burg]

Comment on attachment 353471 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Models/AuditTestCase.js:116
> +            let evaluateResponse = await RuntimeAgent.evaluate.invoke(evaluateArguments);

Nit: better name is evaluateResult or evaluationResult.

> Source/WebInspectorUI/UserInterface/Views/AuditTestCaseContentView.js:109
> +                    durationElement.textContent = Number.secondsToString((metadata.endTimestamp - metadata.startTimestamp) / 1000);

The .00 after every duration is kind of ugly. I thought that Number.secondsToString would have elided that rather than add fake precision.

> Source/WebInspectorUI/UserInterface/Views/AuditTestContentView.css:59
> +    --metadata-width: 60px;

This could be brittle for localization, so we have to fix later.

[Devin Rousso]

Comment on attachment 353471 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Models/AuditTestCase.js:116
>> +            let evaluateResponse = await RuntimeAgent.evaluate.invoke(evaluateArguments);
> 
> Nit: better name is evaluateResult or evaluationResult.

I didn't want to use "result" because the returned object has a "result" member, as `evaluateResult.result` seems "ugly".

>> Source/WebInspectorUI/UserInterface/Views/AuditTestCaseContentView.js:109
>> +                    durationElement.textContent = Number.secondsToString((metadata.endTimestamp - metadata.startTimestamp) / 1000);
> 
> The .00 after every duration is kind of ugly. I thought that Number.secondsToString would have elided that rather than add fake precision.

I'll do a followup for this.

>> Source/WebInspectorUI/UserInterface/Views/AuditTestContentView.css:59
>> +    --metadata-width: 60px;
> 
> This could be brittle for localization, so we have to fix later.

Correct me if I'm wrong, but isn't the "%" symbol the same in all localizations?  Same with numbers?

[Blaze Burg]

(In reply to Devin Rousso from comment #10)
> Comment on attachment 353471 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=353471&action=review
> 
> >> Source/WebInspectorUI/UserInterface/Models/AuditTestCase.js:116
> >> +            let evaluateResponse = await RuntimeAgent.evaluate.invoke(evaluateArguments);
> > 
> > Nit: better name is evaluateResult or evaluationResult.
> 
> I didn't want to use "result" because the returned object has a "result"
> member, as `evaluateResult.result` seems "ugly".
> 
> >> Source/WebInspectorUI/UserInterface/Views/AuditTestCaseContentView.js:109
> >> +                    durationElement.textContent = Number.secondsToString((metadata.endTimestamp - metadata.startTimestamp) / 1000);
> > 
> > The .00 after every duration is kind of ugly. I thought that Number.secondsToString would have elided that rather than add fake precision.
> 
> I'll do a followup for this.
> 
> >> Source/WebInspectorUI/UserInterface/Views/AuditTestContentView.css:59
> >> +    --metadata-width: 60px;
> > 
> > This could be brittle for localization, so we have to fix later.
> 
> Correct me if I'm wrong, but isn't the "%" symbol the same in all
> localizations?  Same with numbers?

No. For example, Arabic uses different number glyphs, which tend to be wider. Percentages also are locale dependent even for Indic numbers.

[WebKit Commit Bot]

Comment on attachment 353471 [details]
Patch

Clearing flags on attachment: 353471

Committed r237644: <https://trac.webkit.org/changeset/237644>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.