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

Status: RESOLVED FIXED

(Tree Management :: Perfherder, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

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

we should remove 'subtests' when we mouse over the specific alert.

Comment 1

[William Lachance (:wlach)]

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)

Comment 2

[William Lachance (:wlach)]

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.

Comment 3

[GitHub Autolander Bot]

Attachment: [treeherder] crosscent:bug1262981 > mozilla:master

Comment 4

[crosscent]

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!

Comment 5

[William Lachance (:wlach)]

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!

Comment 6

[crosscent]

(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.

Comment 7

[William Lachance (:wlach)]

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. :)

Comment 8

[crosscent]

(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

[Treeherder GitHub Bugbot]

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 10

[William Lachance (:wlach)]

Comment on attachment 8759873 [details] [review]
[treeherder] crosscent:bug1262981 > mozilla:master

r+ now, thank you!