Bug 201105 – [results.webkit.org Timline] Add symbols to the timeline dot

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Zhifei Fang]

Add symbols to the timeline dot, make them more meaningful

[Zhifei Fang]

Created attachment 377180 [details]
Patch

[Zhifei Fang]

Created attachment 377183 [details]
Symbols

[Zhifei Fang]

Created attachment 377184 [details]
Symbols

[Ryosuke Niwa]

Comment on attachment 377180 [details]
Patch

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

> Tools/resultsdbpy/resultsdbpy/view/static/js/timeline.js:531
> +            failed: 'X',

Can we use ✘ or ❌ instead?

> Tools/resultsdbpy/resultsdbpy/view/static/js/timeline.js:563
> -                        if (data.stats[`tests${willFilterExpected ? '_unexpected_' : '_'}${type}`] > 0)
> +                        if (data.stats[`tests${willFilterExpected ? '_unexpected_' : '_'}${type}`] > 0) {

Can data.stats be negative? Otherwise we should be omitting > 0 per:
https://webkit.org/code-style-guidelines/#zero-null

[Ryosuke Niwa]

Thanks for this patch!

[Ryosuke Niwa]

I'd refrain from r+ since I don't know this codebase yet.

[Ling Ho]

The content of attachment 377183 [details] has been deleted

[Ling Ho]

The content of attachment 377184 [details] has been deleted

[Jonathan Bedard]

(In reply to Ryosuke Niwa from comment #4)
> Comment on attachment 377180 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=377180&action=review
> 
> > Tools/resultsdbpy/resultsdbpy/view/static/js/timeline.js:531
> > +            failed: 'X',
> 
> Can we use ✘ or ❌ instead?

I would prefer ✘ so we don't have and colored unicode elements.

> 
> > Tools/resultsdbpy/resultsdbpy/view/static/js/timeline.js:563
> > -                        if (data.stats[`tests${willFilterExpected ? '_unexpected_' : '_'}${type}`] > 0)
> > +                        if (data.stats[`tests${willFilterExpected ? '_unexpected_' : '_'}${type}`] > 0) {
> 
> Can data.stats be negative? Otherwise we should be omitting > 0 per:
> https://webkit.org/code-style-guidelines/#zero-null

[Jonathan Bedard]

Comment on attachment 377180 [details]
Patch

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

> Tools/resultsdbpy/resultsdbpy/view/static/js/timeline.js:529
> +        const symbolMap = {

We actually generate the legend in this file ( Legend(...) ) Can we define these globally and fix the scale in this patch too?

[Zhifei Fang]

(In reply to Jonathan Bedard from comment #10)
> Comment on attachment 377180 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=377180&action=review
> 
> > Tools/resultsdbpy/resultsdbpy/view/static/js/timeline.js:529
> > +        const symbolMap = {
> 
> We actually generate the legend in this file ( Legend(...) ) Can we define
> these globally and fix the scale in this patch too?

Will do

[EWS Watchlist]

Comment on attachment 377180 [details]
Patch

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

New failing tests:
js/error-should-not-strong-reference-global-object.html

[EWS Watchlist]

Created attachment 377194 [details]
Archive of layout-test-results from ews215 for win-future

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

[Zhifei Fang]

Created attachment 377272 [details]
Patch

[Jonathan Bedard]

Comment on attachment 377272 [details]
Patch

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

Looks really good! The only changes remaining are changing the x style (Ryosuke asked for that, I think the unicode x looks better) and making dots un-selectable.

Great work!

> Tools/resultsdbpy/resultsdbpy/view/static/library/css/webkit.css:2156
> +  left: calc(0px - var(--tinySize));

I don't think that we want dots to be selectable.

[Zhifei Fang]

Created attachment 377277 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 377277 [details]
Patch

Clearing flags on attachment: 377277

Committed r249124: <https://trac.webkit.org/changeset/249124>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/54727835>