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

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Chris Manchester (limited bugmail, email directly)]

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.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

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

Comment 2

[Andrew Halberstadt [:ahal]]

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.

Comment 3

[Chris Manchester (limited bugmail, email directly)]

(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

Comment 4

[Chris Manchester (limited bugmail, email directly)]

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

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

Comment 5

[Chris Manchester (limited bugmail, email directly)]

 https://hg.mozilla.org/build/mozharness/rev/4a80b712cf05

Comment 6

[Pete Moore [:pmoore][:pete]]

Something here landed in production today: https://wiki.mozilla.org/ReleaseEngineering/Maintenance#Reconfigs_.2F_Deployments