-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Bug 1623601 - Add visual metrics tasks to nightly browsertime tests. #9248
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach is the right one!
I suggested a nit, but I don't think it's a big deal because we're just handling 2 different task kinds.
@@ -56,6 +56,6 @@ def filter(task, parameters): | |||
@_target_task('browsertime') | |||
def target_tasks_raptor(full_task_graph, parameters, graph_config): | |||
def filter(task, parameters): | |||
return task.kind == 'browsertime' | |||
return task.kind == 'browsertime' or task.kind == 'visual-metrics' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python nit: This can be simplified into
return task.kind == 'browsertime' or task.kind == 'visual-metrics' | |
return task.kind in ('browsertime', 'visual-metrics') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks!
@JohanLorenzo I've fixed the nit issue, and it's ready for landing when the PR checks are done. Thanks for the review! |
Codecov Report
@@ Coverage Diff @@
## master #9248 +/- ##
=========================================
Coverage 19.49% 19.49%
Complexity 511 511
=========================================
Files 329 329
Lines 13155 13155
Branches 1748 1748
=========================================
Hits 2565 2565
Misses 10364 10364
Partials 226 226 Continue to review full report at Codecov.
|
Failure is unrelated to the patch. Landing. |
This patch adds the visual-metrics tasks to the nightly browsertime cron tests. They are currently being removed because they don't have a
browsertime
kind.