1144907 - Marionette's gather_debug shouldn't leave calling code in content scope if it didn't start there

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Chris Manchester (limited bugmail, email directly)]

We can use using_context so callers in chrome scope can be sure their tearDown code runs in the correct scope when a test is skipped or fails.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: bz://1144907/chmanchester

/r/5673 - Bug 1144907 - Restore previous context after calling marionette's gather_debug.;r=davehunt

Pull down this commit:

hg pull review -r 7dde4d7281c39f7e39e0eaf7dfca1448fd25c172

Comment 2

[Chris Manchester (limited bugmail, email directly)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=321f37f2d138

Comment 3

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I'm not familiar with MozReview. I've left a comment. Could you include the Gip tests (gaia-ui-test-accessibility,gaia-ui-test-unit,gaia-ui-test-functional-1,gaia-ui-test-functional-2,gaia-ui-test-functional-3) in your try run?

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8579657 [details]
MozReview Request: bz://1144907/chmanchester

https://reviewboard.mozilla.org/r/5671/#review4643

::: testing/marionette/client/marionette/runner/base.py
(Diff revision 1)
> -                    marionette.set_context(marionette.CONTEXT_CONTENT)
> +                    with marionette.using_context(marionette.CONTEXT_CONTENT):

Can we not combine these into a single using_context?

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Hm, how is this related to bug 1141519? David, maybe you can give an answer before Chris comes online later today?

Comment 6

[David Burns :automatedtester]

They are not related. Since we are still using content this patch would still hit the issue that we see in bug 1141519

Comment 7

[Chris Manchester (limited bugmail, email directly)]

(In reply to Dave Hunt (:davehunt) from comment #3)
> I'm not familiar with MozReview. I've left a comment. Could you include the
> Gip tests
> (gaia-ui-test-accessibility,gaia-ui-test-unit,gaia-ui-test-functional-1,gaia-
> ui-test-functional-2,gaia-ui-test-functional-3) in your try run?

Certainly: https://treeherder.mozilla.org/#/jobs?repo=try&revision=69ab6eafda85

Comment 8

[Chris Manchester (limited bugmail, email directly)]

https://reviewboard.mozilla.org/r/5671/#review4677

> Can we not combine these into a single using_context?

It seems that getting a screenshot is really intended to work in chrome, and getting the page source is really intended to work in content, so if we don't have using_context for each either callers in chrome or callers in content aren't going to get what they expect.

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8579657 [details]
MozReview Request: bz://1144907/chmanchester

https://reviewboard.mozilla.org/r/5671/#review4687

Ship It!

Comment 10

[Chris Manchester (limited bugmail, email directly)]

Try in comment 7 and comment 2. The jobs that failed in comment 7 don't run this code.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f709311934dc

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f709311934dc

Comment 14

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1144907 - Restore previous context after calling marionette's gather_debug.;r=davehunt