Titles in comparison and graphs view should reflect content

RESOLVED FIXED

Status

Tree Management
Perfherder
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: wlach, Assigned: kapy)

Tracking

Details

Attachments

(1 attachment)

Currently perfherder has three views: a "graphs" view (get an overview of a particular performance series), a "compare all" view (get an overview of the differences between two revisions on all test suites), and a "compare subtest" view (get the differences between two revisions on the subtests in a particular suite)

Recently we modified the subtest comparison view so the document title (what you see in each tab) reflects the content. E.g.:

https://treeherder.allizom.org/perf.html#/comparesubtest?originalProject=mozilla-central&newProject=mozilla-central&originalRevision=a0787486ecf5&newRevision=eca7c9330539&originalSignature=34e2d9ad9aef8c3f8cd5e6918b228f71403416d5&newSignature=34e2d9ad9aef8c3f8cd5e6918b228f71403416d5

Note how the document title says "windows7-32 tp5o subtest summary". We should do the same for the graphs and full comparison views which currently only say (unhelpfully) "Perfherder".

Full comparison example:

https://treeherder.allizom.org/perf.html#/compare?originalProject=mozilla-central&originalRevision=a0787486ecf5&newProject=mozilla-central&newRevision=eca7c9330539

Graphs example:

https://treeherder.allizom.org/perf.html#/graphs?timerange=1209600&series=%257B%2522project%2522%253A%2522mozilla-central%2522%252C%2522signature%2522%253A%252234e2d9ad9aef8c3f8cd5e6918b228f71403416d5%2522%252C%2522visible%2522%253Atrue%257D&zoom=%7B%7D

It's easy to set the document title in Javascript. All you need is to set window.document.title. You can see the existing example for compare subtests here: https://github.com/mozilla/treeherder-ui/blob/master/webapp/app/js/compareperf.js#L229

For the full comparison, we'll do something similar. I think this makes sense a start: originalRevision (originalBranch) to newRevision (newBranch). The code you need to modify is here:

https://github.com/mozilla/treeherder-ui/blob/master/webapp/app/js/compareperf.js#L94

The data you need is in $scope, you should see that below:

https://github.com/mozilla/treeherder-ui/blob/master/webapp/app/js/compareperf.js#L112

For the graphs view, we could potentially have many different graphs at the same time. For simplicity, let's just use the first one ("series") and append a " and others" if there is more than one. For the name, let's use "<testname> <platform> (branchname)". For example:

tp5o summary opt windows7-32 (mozilla-central)

Again, the information needed to generate this should be in $scope, in this case in the seriesList variable. You can see us manipulating it here:

https://github.com/mozilla/treeherder-ui/blob/master/webapp/app/js/graphs.js#L420

It probably makes the most sense to change updateURL to do this, since it's a central place that is called whenever this type of state changes:

https://github.com/mozilla/treeherder-ui/blob/master/webapp/app/js/graphs.js#L417

Since that method is no longer just used for updating the URL though, we should change it to "updateDocument".
(Assignee)

Comment 1

3 years ago
Will, I would like to work on this project.
Ok, assigned!
Assignee: nobody → kpsingh201091
(Assignee)

Comment 3

3 years ago
Created attachment 8604769 [details] [review]
Corrected titles in graph and comparison views

Check my pull request !!
Attachment #8604769 - Flags: review?(wlachance)

Comment 4

3 years ago
Commit pushed to master at https://github.com/mozilla/treeherder-ui

https://github.com/mozilla/treeherder-ui/commit/bc6ba504c266054095adac0ac29d9a3699b226d0
Bug 1163580 - Titles in comparison and graphs view should reflect content
Comment on attachment 8604769 [details] [review]
Corrected titles in graph and comparison views

Thanks!! Some minor formatting issues, which I just fixed myself before merging. https://github.com/mozilla/treeherder-ui/commit/bc6ba504c266054095adac0ac29d9a3699b226d0
Attachment #8604769 - Flags: review?(wlachance) → review+
Realized after pushing that I missed an edge case where the series list had zero length in my review.

Did a quick followup: https://github.com/mozilla/treeherder-ui/commit/acaa8785ddee92c1e017efd69b34d7f19c5c8a3d
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.