jetperf needs TBPL status bit

RESOLVED FIXED

Status

Release Engineering
General Automation
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Jeff Hammel, Assigned: Jeff Hammel)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mozharness][jetpack+talos])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Currently, jetperf.py doesn't report TBPL status.  It should.

One implementation would be to just stick a TBPL success here:

http://hg.mozilla.org/build/mozharness/file/508ec5a03b42/scripts/jetperf.py#l197

However, this doesn't account for the baseline results, assuming we
want to run both in production.  

Alternately, we could add another action, tbpl_status, that reports
"Good" if both files are found and "Bad" if either of them are
missing.
(Assignee)

Updated

6 years ago
Whiteboard: [mozharness]
(Assignee)

Updated

6 years ago
Blocks: 720901
Whiteboard: [mozharness] → [mozharness][jetpack+talos]
I don't know what you mean here...What kind of status do you need to report? Is the return code of the process not sufficient?

Comment 2

6 years ago
We're probably going to put something like this in:
http://hg.mozilla.org/build/mozharness/file/0a36a198ec6a/scripts/peptest.py#l196
well, I still don't know what to do.
Assignee: nobody → jhammel
(Assignee)

Comment 4

6 years ago
Created attachment 630319 [details] [diff] [review]
add TBPL status

unsure if this is the best going forward
Attachment #630319 - Flags: review?(aki)

Comment 5

6 years ago
Comment on attachment 630319 [details] [diff] [review]
add TBPL status

This looks good.
Running unit.sh:

scripts/jetperf.py:15: 'TBPL_FAILURE' imported but unused
scripts/jetperf.py:239: undefined name 'TBPL_WARNING'
scripts/jetperf.py:242: undefined name 'TBPL_WARNING'
scripts/jetperf.py:242: local variable 'tbbpl_status' is assigned to but never used
### Running pylint
scripts/jetperf.py:239: [E, JetPerf.report_tbpl_status] Undefined variable 'TBPL_WARNING'
scripts/jetperf.py:242: [E, JetPerf.report_tbpl_status] Undefined variable 'TBPL_WARNING'

If you import TBPL_WARNING instead of TBPL_FAILURE, I think this will work.
Also s,tbbpl,tbpl

We can also probably remove report-tbpl-status from the default actions, if we're making the script developer friendly out of the box.
Since it's its own action, I think I'd like to see some message, like

        tbpl_status = TBPL_WARNING
        self.warning("%s doesn't exist after running tests!" % filename)

r=me with those changes.
Attachment #630319 - Flags: review?(aki) → review+
(Assignee)

Comment 6

6 years ago
One additional thing to consider....there are several uses of self.fatal in jetperf.py.  I'm not sure how these are reconciled with TBPL status.

Comment 7

6 years ago
Pretty sure we'll go red when we call self.fatal().
(Assignee)

Comment 8

6 years ago
Created attachment 630602 [details] [diff] [review]
add TBPL status
Attachment #630319 - Attachment is obsolete: true
Attachment #630602 - Flags: review?(aki)
(Assignee)

Comment 9

6 years ago
(In reply to Aki Sasaki [:aki] from comment #5)
> Comment on attachment 630319 [details] [diff] [review]
> add TBPL status
> 
> This looks good.
> Running unit.sh:
> 
> scripts/jetperf.py:15: 'TBPL_FAILURE' imported but unused
> scripts/jetperf.py:239: undefined name 'TBPL_WARNING'
> scripts/jetperf.py:242: undefined name 'TBPL_WARNING'
> scripts/jetperf.py:242: local variable 'tbbpl_status' is assigned to but
> never used
> ### Running pylint
> scripts/jetperf.py:239: [E, JetPerf.report_tbpl_status] Undefined variable
> 'TBPL_WARNING'
> scripts/jetperf.py:242: [E, JetPerf.report_tbpl_status] Undefined variable
> 'TBPL_WARNING'
> 
> If you import TBPL_WARNING instead of TBPL_FAILURE, I think this will work.
> Also s,tbbpl,tbpl

Good catch, fixed.

> We can also probably remove report-tbpl-status from the default actions, if
> we're making the script developer friendly out of the box.
> Since it's its own action, I think I'd like to see some message, like
> 
>         tbpl_status = TBPL_WARNING
>         self.warning("%s doesn't exist after running tests!" % filename)
> 
> r=me with those changes.

Fixed, hopefully to your liking

Comment 10

6 years ago
Comment on attachment 630602 [details] [diff] [review]
add TBPL status

Looks good!
Attachment #630602 - Flags: review?(aki) → review+
(Assignee)

Comment 11

6 years ago
pushed: http://hg.mozilla.org/build/mozharness/rev/7d490d0e3466
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.