Don't use properties of the output parser directly to determine failures in b2g_emulator_unittest.py

RESOLVED FIXED

Status

Release Engineering
Mozharness
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Chris Manchester (Offline Jun 23-26), Assigned: Chris Manchester (Offline Jun 23-26))

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

b2g_emulator_unittest.py checks for a running emulator if it detects a failure based on the fail_count, crashed, and leaked properties of the outputparser. This isn't a part of the output parser I'd like to re-implement for the StructuredOutputParser -- I think checking whether tbpl_status != TBPL_SUCCESS will happen under the same circumstances.
(Assignee)

Updated

3 years ago
Blocks: 1068138
(Assignee)

Comment 1

3 years ago
Created attachment 8493335 [details] [diff] [review]
Determine a failure in b2g_emulator_unittest.py based on the tbpl_status, not attributes of the parser.

By my reading, parser.leaked or parser.crashed always implies tbpl_status of at least TBPL_WARNING. I'll test this out on ash tonight.
Attachment #8493335 - Flags: review?(ahalberstadt)
(Assignee)

Updated

3 years ago
Assignee: nobody → cmanchester
Status: NEW → ASSIGNED
Comment on attachment 8493335 [details] [diff] [review]
Determine a failure in b2g_emulator_unittest.py based on the tbpl_status, not attributes of the parser.

Review of attachment 8493335 [details] [diff] [review]:
-----------------------------------------------------------------

This makes sense to me, r+. Though yeah, an ash run would be a good idea.

::: scripts/b2g_emulator_unittest.py
@@ +407,5 @@
> +
> +        qemu = os.path.join(dirs['abs_work_dir'], 'qemu.log')
> +        if os.path.isfile(qemu):
> +            self.copyfile(qemu, os.path.join(env['MOZ_UPLOAD_DIR'],
> +                                             os.path.basename(qemu)))

Seems unnecessary to move the qemu copying stuff around.
Attachment #8493335 - Flags: review?(ahalberstadt) → review+
(Assignee)

Comment 3

3 years ago
(In reply to Andrew Halberstadt [:ahal] from comment #2)
> Comment on attachment 8493335 [details] [diff] [review]
> Determine a failure in b2g_emulator_unittest.py based on the tbpl_status,
> not attributes of the parser.
> 
> Review of attachment 8493335 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This makes sense to me, r+. Though yeah, an ash run would be a good idea.
> 
> ::: scripts/b2g_emulator_unittest.py
> @@ +407,5 @@
> > +
> > +        qemu = os.path.join(dirs['abs_work_dir'], 'qemu.log')
> > +        if os.path.isfile(qemu):
> > +            self.copyfile(qemu, os.path.join(env['MOZ_UPLOAD_DIR'],
> > +                                             os.path.basename(qemu)))
> 
> Seems unnecessary to move the qemu copying stuff around.

I think the diff is misleading because the part I moved is bigger than the part that stayed. The patch moves the block that checks for logcat and runs ps on failure after evaluate_parser and leaves the qemu stuff above the call to evaluate_parser in case the sequence is sensitive. Here it is on ash, ought to be green (I'll push a crashy version later):

https://treeherder.mozilla.org/ui/#/jobs?repo=ash&revision=80e3afe71657
(Assignee)

Comment 4

3 years ago
This push with a crash shows we run ps after a red mochitest run:

https://treeherder.mozilla.org/ui/#/jobs?repo=ash&revision=5224561bbf4f
(Assignee)

Comment 5

3 years ago
 https://hg.mozilla.org/build/mozharness/rev/4a80b712cf05
Something here landed in production today: https://wiki.mozilla.org/ReleaseEngineering/Maintenance#Reconfigs_.2F_Deployments
(Assignee)

Updated

3 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.