Bug 238809 – [resultsdb] Expose reported flakiness information in the UI

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Angelos Oikonomopoulos]

[resultsdb] Expose reported flakiness information in the UI

[Angelos Oikonomopoulos]

Created attachment 456707 [details]
Patch

[Angelos Oikonomopoulos]

FWIW I tried using the dotWidth as the maxWidth to CanvasRenderingContext2D.fillText() but that didn't seem to help (and I suspect the resulting text would be unreadably small in any case).

[Jonathan Bedard]

Comment on attachment 456707 [details]
Patch

Generally looks good, although I'd like to see it before landing. Do you have screen shots?

[Angelos Oikonomopoulos]

Created attachment 456742 [details]
Screenshot of the suites view with Show number of flaky tests on

This is a screenshot after browsing to /suites (with some randomly generated test results inserted) and turning on 'Show number of flaky tests' (defaults to off).

[Angelos Oikonomopoulos]

Created attachment 456743 [details]
Screenshot of the search view with Show test flakiness on

Visiting /?suite=javascriptcore-tests&test=stress/test1 and setting 'Show test flakiness' to on (defaults to off).

[Angelos Oikonomopoulos]

Created attachment 456914 [details]
Patch

[Angelos Oikonomopoulos]

The new version of the patch no longer expects a nested dict, to match the change in https://bugs.webkit.org/show_bug.cgi?id=238806.

I've been wondering whether it makes sense to have a radio button for the dot tags, so that it's both always apparent what piece of information one is looking at (so there would be no need for additional text -- e.g. the current patch prepends 'fl ' to the number of flaky tests). This would also mean that the tag text can't get too long and start overlapping with adjacent tags.

[Jonathan Bedard]

Comment on attachment 456914 [details]
Patch

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

I would like to get https://bugs.webkit.org/show_bug.cgi?id=238806 landed first so we can deploy this on the staging instance before landing

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/timeline.js:733
> +                    tag.push('fl ' + data.details.number_of_flaky_tests);

This ends up being a bit long in the UI...I suppose it's hidden behind a switch, so it's not a huge deal,

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/timeline.js:1171
> +                <label>Show number of flaky tests</label>

The name of this label is a bit long. Maybe "Show number of flakes"? Or just "Number of flakes"?

[Angelos Oikonomopoulos]

Created attachment 457246 [details]
Patch

[Angelos Oikonomopoulos]

(In reply to Jonathan Bedard from comment #8)
> Comment on attachment 456914 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=456914&action=review
> 
> I would like to get https://bugs.webkit.org/show_bug.cgi?id=238806 landed
> first so we can deploy this on the staging instance before landing
> 
> > Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/timeline.js:733
> > +                    tag.push('fl ' + data.details.number_of_flaky_tests);
> 
> This ends up being a bit long in the UI...I suppose it's hidden behind a
> switch, so it's not a huge deal,

Yah, I don't have a good solution for that, other than perhaps the radio button mentioned above.

> > Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/timeline.js:1171
> > +                <label>Show number of flaky tests</label>
> 
> The name of this label is a bit long. Maybe "Show number of flakes"? Or just
> "Number of flakes"?

Changed to "Number of flakes".

[Radar WebKit Bug Importer]

<rdar://problem/91630047>

[Jonathan Bedard]

Comment on attachment 457246 [details]
Patch

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

> Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/timeline.js:-727
> -                return drawDot(context, x, y, false, tag ? tag : null, symbol, false, color);

New code will show 0, which is not what we want. Probably need an extra check when building tag list to throw out zeros.

[Angelos Oikonomopoulos]

Created attachment 457615 [details]
Patch

[Angelos Oikonomopoulos]

(In reply to Jonathan Bedard from comment #12)
> Comment on attachment 457246 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=457246&action=review
> 
> > Tools/Scripts/libraries/resultsdbpy/resultsdbpy/view/static/js/timeline.js:-727
> > -                return drawDot(context, x, y, false, tag ? tag : null, symbol, false, color);
> 
> New code will show 0, which is not what we want. Probably need an extra
> check when building tag list to throw out zeros.

Oops. Hopefully fixed now.

[Angelos Oikonomopoulos]

Ping.

[Jonathan Bedard]

Finally got a chance to deploy this on our staging instance today, looks good to go! Change will need to be re-uploaded with a commit message instead of a changelog, but the code itself looks good!

[Angelos Oikonomopoulos]

Pull request: https://github.com/WebKit/WebKit/pull/2051

[EWS]

Committed 252114@main (654dea109d94): <https://commits.webkit.org/252114@main>

Reviewed commits have been landed. Closing PR #2051 and removing active labels.

[Angelos Oikonomopoulos]

(In reply to Jonathan Bedard from comment #16)
> Finally got a chance to deploy this on our staging instance today, looks
> good to go! Change will need to be re-uploaded with a commit message instead
> of a changelog, but the code itself looks good!

Re-uploaded, thanks for testing it!

[Jonathan Bedard]

(In reply to Angelos Oikonomopoulos from comment #19)
> (In reply to Jonathan Bedard from comment #16)
> > Finally got a chance to deploy this on our staging instance today, looks
> > good to go! Change will need to be re-uploaded with a commit message instead
> > of a changelog, but the code itself looks good!
> 
> Re-uploaded, thanks for testing it!

The problem is the ChangeLog. WebKit no longer has changelogs, only commit messages.

[Jonathan Bedard]

Landed in 252114@main

[Jonathan Bedard]

This has been deployed!