build_metrics alerts have a link for subtests, but we have no subtests

RESOLVED FIXED

Status

Tree Management
Perfherder
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jmaher, Assigned: crosscent)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
here is a link:
https://treeherder.mozilla.org/perf.html#/alerts?id=760

we should remove 'subtests' when we mouse over the specific alert.
This is easy, we just need to add the `has_subtests` property to the signature we return on the alertsummary endpoint, then use that to determine whether to show the subtests link.

(we may also want to do this for some other places inside the perfherder client ui, such as the graphs view)
To test this, you'll need the full-blown vagrant environment set up with some test performance data. Once you've set up vagrant, run the following commands:

./manage.py create_test_perf_data # will require confirmation
./manage.py generate_alerts --project mozilla-central
./manage.py runserver # will need to re-run this after making changes

After that, you should be able to browse to http://local.treeherder.mozilla.org/perf.html#/alerts . There will be a warning about missing revision data on the alerts page-- just ignore that for the purposes of this bug.
Assignee: nobody → crosscent

Comment 3

2 years ago
Created attachment 8759873 [details] [review]
[treeherder] crosscent:bug1262981 > mozilla:master
(Assignee)

Comment 4

2 years ago
Will, I removed the link to subtests when no subtests are present, and also filtered them out from graph view list. I also added a new method getSubtestsURL so it matches how the graph link is done. If that's not really optimal, I can change it back!
(Assignee)

Updated

2 years ago
Attachment #8759873 - Flags: review?(wlachance)
Comment on attachment 8759873 [details] [review]
[treeherder] crosscent:bug1262981 > mozilla:master

Good call on creating a method for generating the URL. Some comments in the PR, but this is very close!
Attachment #8759873 - Flags: review?(wlachance) → review-
(Assignee)

Comment 6

2 years ago
(In reply to William Lachance (:wlach) from comment #5)
> Good call on creating a method for generating the URL. Some comments in the
> PR, but this is very close!

Ha! Thanks Will for the comments! I have made the changes. The map function was super useful!

I've tried to use a one line if-else statement for parameters that needed to be checked, but that would make the line extremely long, so I kept the original if-else statement.
(Assignee)

Updated

2 years ago
Attachment #8759873 - Flags: review- → review?(wlachance)
Comment on attachment 8759873 [details] [review]
[treeherder] crosscent:bug1262981 > mozilla:master

Hey, I noticed a few last small problems. Could you fix them quickly? I'd be happy to merge after. :)
Attachment #8759873 - Flags: review?(wlachance) → review-
(Assignee)

Comment 8

2 years ago
(In reply to William Lachance (:wlach) from comment #7)
> Hey, I noticed a few last small problems. Could you fix them quickly? I'd be
> happy to merge after. :)

Hey Will! Sorry I had to go to the office for the past days, and do some code crunches. I will update it later today!

Comment 9

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

https://github.com/mozilla/treeherder/commit/a4c1aaf4936c6eafad6ee7d510ead0d4a2e4ce9e
Bug 1262981 - Filter subtests in graph view and show link when subtests are present (#1560)
Comment on attachment 8759873 [details] [review]
[treeherder] crosscent:bug1262981 > mozilla:master

r+ now, thank you!
Attachment #8759873 - Flags: review- → review+
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.