1368674 - remove simpleTest functionality

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, enhancement)

Description

[David Burns :automatedtester]

This is a hangover from b2g and is not longer needed and is just cluttering up the code base.

Comment 1

[David Burns :automatedtester]

Attachment: Bug 1368674 - Remove simpleTest functionality from Marionette.

This is a remanent of the B2G code for injecting Mochitest style tests
into Gecko. This is no longer used by anything and is now dead code.

Review commit: https://reviewboard.mozilla.org/r/144198/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/144198/

Comment 2

[David Burns :automatedtester]

Attachment: Bug 1368674 - Remove JS Test support in Marionette Harness

Review commit: https://reviewboard.mozilla.org/r/144200/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/144200/

Comment 3

[David Burns :automatedtester]

Attachment: Bug 1368674 - Remove setTestName functionality from Marionette

setTestName was used for logging which test was being run for the
JS Tests used in B2G.

Review commit: https://reviewboard.mozilla.org/r/144202/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/144202/

Comment 4

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

Comment on attachment 8872657 [details]
Bug 1368674 - Remove simpleTest functionality from Marionette.

https://reviewboard.mozilla.org/r/144198/#review147910

Agreeing with Andreas about that it is sad to see this going away. But it makes sense.

Comment 5

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

Comment on attachment 8872659 [details]
Bug 1368674 - Remove setTestName functionality from Marionette

https://reviewboard.mozilla.org/r/144202/#review147916

Comment 6

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8872658 [details]
Bug 1368674 - Remove JS Test support in Marionette Harness

https://reviewboard.mozilla.org/r/144200/#review147976

I think you can remove more! Like https://dxr.mozilla.org/mozilla-central/source/testing/marionette/harness/marionette_harness/runner/base.py#186 and https://dxr.mozilla.org/mozilla-central/source/testing/marionette/harness/marionette_harness/runner/base.py#609 and update https://dxr.mozilla.org/mozilla-central/source/testing/marionette/harness/marionette_harness/tests/harness_unit/test_marionette_runner.py#285

::: testing/marionette/harness/marionette_harness/marionette_test/testcases.py:63
(Diff revision 1)
>  class JSTest:
>      head_js_re = re.compile(r"MARIONETTE_HEAD_JS(\s*)=(\s*)['|\"](.*?)['|\"];")
>      context_re = re.compile(r"MARIONETTE_CONTEXT(\s*)=(\s*)['|\"](.*?)['|\"];")
>      timeout_re = re.compile(r"MARIONETTE_TIMEOUT(\s*)=(\s*)(\d+);")
>      inactivity_timeout_re = re.compile(r"MARIONETTE_INACTIVITY_TIMEOUT(\s*)=(\s*)(\d+);")

remove this, too?

Comment 7

[David Burns :automatedtester]

Comment on attachment 8872657 [details]
Bug 1368674 - Remove simpleTest functionality from Marionette.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144198/diff/1-2/

Comment 8

[David Burns :automatedtester]

Comment on attachment 8872658 [details]
Bug 1368674 - Remove JS Test support in Marionette Harness

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144200/diff/1-2/

Comment 9

[David Burns :automatedtester]

Comment on attachment 8872659 [details]
Bug 1368674 - Remove setTestName functionality from Marionette

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144202/diff/1-2/

Comment 10

[David Burns :automatedtester]

Comment on attachment 8872658 [details]
Bug 1368674 - Remove JS Test support in Marionette Harness

https://reviewboard.mozilla.org/r/144200/#review147976

> remove this, too?

I deleted a lot more once I went down that rabbit hole.

Comment 11

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8872658 [details]
Bug 1368674 - Remove JS Test support in Marionette Harness

https://reviewboard.mozilla.org/r/144200/#review148384

Comment 12

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

Comment on attachment 8872659 [details]
Bug 1368674 - Remove setTestName functionality from Marionette

https://reviewboard.mozilla.org/r/144202/#review148518

::: testing/marionette/driver.js
(Diff revision 2)
> -  assert.window(this.getCurrentWindow());
> -
> -  let val = cmd.parameters.value;
> -  this.testName = val;
> -  yield this.listener.setTestName({value: val});
> -};

David, before you push this patch series please make sure we do not run into troubles with logging:

1496218610991	Marionette	TRACE	1051 -> [0,5,"setTestName",{"value":"test_navigation.py TestRefresh.test_timeout_error"}]
1496218611003	Marionette	TRACE	1051 <- [1,5,null,{}]
1496218611022	Marionette	TRACE	1051 -> [0,6,"executeScript",{"scriptTimeout":null,"newSandbox":true,"args":[],"filename":"testcases.py","script":"log('TEST-START: /home/worker/workspace/build/tests/marionette/tests/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py:test_timeout_error')","sandbox":"simpletest","line":473}]
MARIONETTE LOG: INFO: TEST-START: /home/worker/workspace/build/tests/marionette/tests/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py:test_timeout_error

Please see the `TEST-START` line which doesn't contain the class name inside the module! When there are different test methods with the same name but attached to different classes we won't be able to differentiate those.

Comment 13

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

Comment on attachment 8872659 [details]
Bug 1368674 - Remove setTestName functionality from Marionette

https://reviewboard.mozilla.org/r/144202/#review148518

> David, before you push this patch series please make sure we do not run into troubles with logging:
> 
> 1496218610991	Marionette	TRACE	1051 -> [0,5,"setTestName",{"value":"test_navigation.py TestRefresh.test_timeout_error"}]
> 1496218611003	Marionette	TRACE	1051 <- [1,5,null,{}]
> 1496218611022	Marionette	TRACE	1051 -> [0,6,"executeScript",{"scriptTimeout":null,"newSandbox":true,"args":[],"filename":"testcases.py","script":"log('TEST-START: /home/worker/workspace/build/tests/marionette/tests/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py:test_timeout_error')","sandbox":"simpletest","line":473}]
> MARIONETTE LOG: INFO: TEST-START: /home/worker/workspace/build/tests/marionette/tests/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py:test_timeout_error
> 
> Please see the `TEST-START` line which doesn't contain the class name inside the module! When there are different test methods with the same name but attached to different classes we won't be able to differentiate those.

Just to add if we loose this it would add a huge burden on me regarding analysis of test failures.

Comment 14

[David Burns :automatedtester]

Comment on attachment 8872659 [details]
Bug 1368674 - Remove setTestName functionality from Marionette

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144202/diff/2-3/

Comment 15

[David Burns :automatedtester]

Comment on attachment 8872657 [details]
Bug 1368674 - Remove simpleTest functionality from Marionette.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144198/diff/2-3/

Comment 16

[David Burns :automatedtester]

Comment on attachment 8872658 [details]
Bug 1368674 - Remove JS Test support in Marionette Harness

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144200/diff/2-3/

Comment 17

[David Burns :automatedtester]

Comment on attachment 8872659 [details]
Bug 1368674 - Remove setTestName functionality from Marionette

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144202/diff/3-4/

Comment 18

[David Burns :automatedtester]

Comment on attachment 8872659 [details]
Bug 1368674 - Remove setTestName functionality from Marionette

https://reviewboard.mozilla.org/r/144202/#review148518

> Just to add if we loose this it would add a huge burden on me regarding analysis of test failures.

Fixed and new try up

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f4f851da483
Remove simpleTest functionality from Marionette. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/837ccbc38bfc
Remove JS Test support in Marionette Harness r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/9aa183c8533e
Remove setTestName functionality from Marionette r=whimboo

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Backed out for flake8 failures.
https://hg.mozilla.org/integration/autoland/rev/18b819dd5afc1b7af098076343487f4e5ef3d08e

https://treeherder.mozilla.org/logviewer.html#?job_id=103830280&repo=autoland

Comment 21

[David Burns :automatedtester]

Comment on attachment 8872659 [details]
Bug 1368674 - Remove setTestName functionality from Marionette

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144202/diff/4-5/

Comment 22

[David Burns :automatedtester]

Comment on attachment 8872657 [details]
Bug 1368674 - Remove simpleTest functionality from Marionette.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144198/diff/3-4/

Comment 23

[David Burns :automatedtester]

Comment on attachment 8872658 [details]
Bug 1368674 - Remove JS Test support in Marionette Harness

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144200/diff/3-4/

Comment 24

[David Burns :automatedtester]

Comment on attachment 8872659 [details]
Bug 1368674 - Remove setTestName functionality from Marionette

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144202/diff/5-6/

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d0202706572
Remove simpleTest functionality from Marionette. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/0d9bb636b9a9
Remove JS Test support in Marionette Harness r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/ff3c813fcdea
Remove setTestName functionality from Marionette r=whimboo

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Backed out for Android test failures.
https://hg.mozilla.org/integration/autoland/rev/bf5e8ffae40db7b2c2afcb1851fb8eb80b98e5b6

https://treeherder.mozilla.org/logviewer.html#?job_id=103854474&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=103854409&repo=autoland

Comment 27

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1d0202706572
https://hg.mozilla.org/mozilla-central/rev/0d9bb636b9a9
https://hg.mozilla.org/mozilla-central/rev/ff3c813fcdea

Comment 28

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I somehow didn't missed this backout in my merge, so that shouldn't have merged to m-c. Backing it out now.

Comment 29

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backout: https://hg.mozilla.org/mozilla-central/rev/aeb3d0ca558f034cbef1c5a68bd07dd738611494

Comment 30

[David Burns :automatedtester]

Attachment: Bug 1368674 - Remove B2G Permissionis handling from Marionette.

The Permissions API was used extensively during B2G to make sure the
state of the browser and permissions for APIs were in the relevant
state. This code is no longer used and can be removed.

Review commit: https://reviewboard.mozilla.org/r/146214/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/146214/

Comment 31

[David Burns :automatedtester]

Comment on attachment 8872657 [details]
Bug 1368674 - Remove simpleTest functionality from Marionette.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144198/diff/4-5/

Comment 32

[David Burns :automatedtester]

Comment on attachment 8872658 [details]
Bug 1368674 - Remove JS Test support in Marionette Harness

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144200/diff/4-5/

Comment 33

[David Burns :automatedtester]

Comment on attachment 8872659 [details]
Bug 1368674 - Remove setTestName functionality from Marionette

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/144202/diff/6-7/

Comment 34

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8874825 [details]
Bug 1368674 - Remove B2G Permissionis handling from Marionette.

https://reviewboard.mozilla.org/r/146214/#review150162

::: commit-message-4acd9:1
(Diff revision 1)
> +Bug 1368674 - Remove B2G Permission handling from Marionette. r?ato

Missing +s in “Permission”.

Comment 35

[David Burns :automatedtester]

Comment on attachment 8874825 [details]
Bug 1368674 - Remove B2G Permissionis handling from Marionette.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/146214/diff/1-2/

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/28b38b97ffbd
Remove simpleTest functionality from Marionette. r=whimboo
https://hg.mozilla.org/integration/autoland/rev/db29fed24bd4
Remove JS Test support in Marionette Harness r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/541fbf25b024
Remove setTestName functionality from Marionette r=whimboo
https://hg.mozilla.org/integration/autoland/rev/98553984eedf
Remove B2G Permissionis handling from Marionette. r=ato

Comment 37

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/28b38b97ffbd
https://hg.mozilla.org/mozilla-central/rev/db29fed24bd4
https://hg.mozilla.org/mozilla-central/rev/541fbf25b024
https://hg.mozilla.org/mozilla-central/rev/98553984eedf