Closed Bug 1368674 Opened 7 years ago Closed 7 years ago

remove simpleTest functionality

Categories

(Remote Protocol :: Marionette, enhancement)

Version 3
enhancement
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: automatedtester, Assigned: automatedtester)

References

Details

Attachments

(4 files)

This is a hangover from b2g and is not longer needed and is just cluttering up the code base.
Blocks: 1306604
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.
Attachment #8872657 - Flags: review?(hskupin) → review+
Comment on attachment 8872659 [details]
Bug 1368674 - Remove setTestName functionality from Marionette

https://reviewboard.mozilla.org/r/144202/#review147916
Attachment #8872659 - Flags: review?(hskupin) → review+
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?
Attachment #8872658 - Flags: review?(mjzffr) → review-
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 on attachment 8872658 [details]
Bug 1368674 - Remove JS Test support in Marionette Harness

https://reviewboard.mozilla.org/r/144200/#review148384
Attachment #8872658 - Flags: review?(mjzffr) → review+
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 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 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
Pushed by dburns@mozilla.com:
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
Pushed by dburns@mozilla.com:
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
I somehow didn't missed this backout in my merge, so that shouldn't have merged to m-c. Backing it out now.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla55 → ---
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”.
Attachment #8874825 - Flags: review?(ato) → review+
Pushed by dburns@mozilla.com:
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
No longer depends on: 1374323
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: