1187469 - Provide alternate tooltip content when "(pushlog, compare)" isn't available

Status: RESOLVED FIXED

(Tree Management :: Perfherder, defect, P4)

Description

[Jonathan French (:jfrench)]

So on stage, I understand we expire perf data after 30 days. If you graph scatter points in perf on stage with a time range greater than that (eg. 60 days, 90 days), the most recent data points which are otherwise valid in shorter time windows, will show no "(pushlog, compare)" string in their tooltips.

If that's correct behavior, should we indicate in the tooltip something else that cues the user? eg.

"(n/a time range)"

or

"(out of time range)"

Or similar.

Comment 1

[Jonathan French (:jfrench)]

Steps to reproduce:
o open https://treeherder.allizom.org/perf.html
o Add Test Data
o select any data and Add+
o click on the rightmost data point (eg. today)
o ...it will correctly show a "(pushlog, compare)" links in the tooltip
o Select "Last 60 days" from the range dropdown
o click on the same rightmost data point
o ...it is absent the "(pushlog, compare)" link

Comment 2

[Jonathan French (:jfrench)]

Attachment: 60dayView

Per screen grab.

Comment 3

[Alex [:moijes12]]

Hi Jonathan

I have attached 2 screen prints with what I understand of the problem:

1. with_pushlog_compare_text.jpg: 
For the span class used to display the pushlog and compare links, the ng-show tag is used with the class set to ""

2. without_pushlog_compare_text.jpg:
For the span class used to display the pushlog and compare links, the ng-show is still used but the class is set to ng-hide.

As I can see, the GraphsCtrl is the controller here. Please can you guide me on fixing this.

Comment 4

[Alex [:moijes12]]

Attachment: Screenshot when pushlog,compare is present

Comment 5

[Alex [:moijes12]]

Attachment: Screenshot when pushlog,compare is not present

Comment 6

[Alex [:moijes12]]

Also, what I see is that, if the pushlog, compare wasn't hidden, then the "Loading Revision" text will be displayed continuously under it for a long time.

Comment 7

[Jonathan French (:jfrench)]

I believe this is part of what handles the links
https://github.com/mozilla/treeherder/blob/655f06b136ddd50cea180b90c9f316bb9b2d67d6/ui/partials/perf/graphsctrl.html#L72-L74

You need to click on the right most data point to reproduce the bug. If you are clicking on other data points that just say "Loading revision..." I suspect you are clicking on data points outside the 30 day cycling window (ie. the left side of the scatter graph in 60,90 day views). This bug is about valid data on the right hand side.

The "Loading revision..." for data which has non (older than 30 days) is a separate bug I think.

You can inspect the dom using the browser inspector to see what is being exposed in the failure case (60,90 day view) on right hand data points. I defer to Will though on how the tooltip how this should be handled, since it may have changed since I entered the bug. Setting Will as mentor :)

Comment 8

[GitHub Autolander Bot]

Attachment: [treeherder] kapy2010:master > mozilla:master

Comment 9

[Kapil Singh [:kapy]]

Attachment: Provided appropriate message when revision info isn't available

Comment 10

[William Lachance (:wlach)]

Comment on attachment 8707415 [details] [review]
Provided appropriate message when revision info isn't available

Hey Kapy, thanks, this is great! Note that there is no longer any need to attach PR's to bugs, now that we're using Autolander. Just set the r? flag on the PR that *it* attaches. Should save you a bit of time (also makes it easier to actually get the patch in tree).

https://github.com/mozilla/autolander

Comment 11

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla/treeherder/commit/0b5e29a73bad551dbdb46ff17573cdc752b8a78a

Comment 12

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/eced62b89d52694e3b7ef82e67c97452dd0b0198
Bug 1187469 - Provide alternate tooltip content when (pushlog, compare) isn't available