Edgar Chen [:edgar] Reporter
	Description • 10 years ago

The feature, 'oop=both/true/false' was removed in bug 1058158, but we still want to have a way to run test script in both OOP and non-OOP mode for bug 926277 (to have the better test coverage). Could we add it back, or maybe makes 'test_container' supporting 'both' (adding same test script twice, one is in-process, the other is OOP)?

	Malini Das [:mdas] - Away, not checking bugmail
	Comment 1 • 10 years ago

(In reply to Edgar Chen [:edgar][:echen] from comment #0)
> The feature, 'oop=both/true/false' was removed in bug 1058158, but we still
> want to have a way to run test script in both OOP and non-OOP mode for bug
> 926277 (to have the better test coverage). Could we add it back, or maybe
> makes 'test_container' supporting 'both' (adding same test script twice, one
> is in-process, the other is OOP)?

test_container=both would work.

	Edgar Chen [:edgar] Reporter
	Comment 5 • 10 years ago

Comment on attachment 8503044 [details] [diff] [review]
Patch, v3

I tested this patch with bug 1079880 (enable cellbroadcast OOP test by setting testcontainer to `both` in manifest), it looks all good in try. 

https://tbpl.mozilla.org/?tree=Try&rev=08059d64b3a7

May I have your review, mdas? thank you.

	Malini Das [:mdas] - Away, not checking bugmail
	Comment 6 • 10 years ago

Comment on attachment 8503044 [details] [diff] [review]
Patch, v3

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

The patch itself looks good, but unfortunately there is a problem with this approach.

When we unload gaia for the in-process tests, the subsequent webapi tests that we run may fail. That's why we introduced the test containers in Bug 1058158. What we should be doing here is adding some logic to check if we finished running an in-process test, and if so, we should restart the b2g process and wait for marionette to come back up. I have similar logic in the patch here: https://bugzilla.mozilla.org/show_bug.cgi?id=1075451, where I added a property to note if the last test used the test container or not. You can probably use this idea and track if the previous test was inprocess or not, and if so, restart b2g and wait for marionette to come up.

If you aren't up for continuing writing this patch, I can take it over, but I have quite a few other priorities at the moment, so it may take me a while to get to!

	Ethan Tseng [:ethan]
	Comment 7 • 10 years ago

Bug 1032111 builds the first test case for RTSP streaming.
Due to the limitation of current RTSP framework, the test script has to run in OOP mode.
Thus it depends on the fix of this bug.

	Edgar Chen [:edgar] Reporter
	Comment 8 • 10 years ago

Hi :jgriffin, 
We would like to enable more test running in OOP mode, e.g. bug 1032111 and bug 926277, but we encounter an issue that once device ran into in-process test, there is no way to run OOP test again (because system app was unloaded for in-process test). My initial idea was re-ordering the test: running all OOP test and then in-process test. But reviewer suggests another approach: restart the b2g process if the following test is OOP one (see comment #6). Do you know who can help on this (sorry, I am not familiar with marionette-framework code enough :()? And is it acceptable that we use re-ordering approach first and file a follow-up bug for restart-b2g approach?
Thank you.

	Edgar Chen [:edgar] Reporter
	Comment 9 • 10 years ago

(In reply to Edgar Chen [:edgar][:echen] from comment #8)
> Hi :jgriffin, 
> We would like to enable more test running in OOP mode, e.g. bug 1032111 and
> bug 926277, but we encounter an issue that once device ran into in-process
> test, there is no way to run OOP test again (because system app was unloaded
> for in-process test). My initial idea was re-ordering the test: running all
> OOP test and then in-process test. But reviewer suggests another approach:
> restart the b2g process if the following test is OOP one (see comment #6).
> Do you know who can help on this (sorry, I am not familiar with
> marionette-framework code enough :()? And is it acceptable that we use
> re-ordering approach first and file a follow-up bug for restart-b2g approach?
> Thank you.

Or keeps current MNW for tests running in in-process mode and add a new task, e.g. MNW-OOP, for tests running in OOP mode. I can imagine that marionette.py probably needs a new options and it seems much easier to add a new task in taskcluster.

	Ethan Tseng [:ethan]
	Comment 10 • 10 years ago

(In reply to Edgar Chen [:edgar][:echen] from comment #9)
> Or keeps current MNW for tests running in in-process mode and add a new
> task, e.g. MNW-OOP, for tests running in OOP mode. I can imagine that
> marionette.py probably needs a new options and it seems much easier to add a
> new task in taskcluster.
Sounds like a good option!

	Jonathan Griffin (:jgriffin)
	Comment 11 • 10 years ago

Any of these approaches would be fine; certainly the easiest would be the test reordering idea.  Another option is to have in in-process tests run in a different Gaia app that isn't OOP, and then you could switch back and forth without any issues.

I think the worst solution is probably to restart the B2G process each time the harness switches from running a non-OOP test to an OOP test, because that operation is exceedingly slow on the test slaves, and this could potentially add quite a bit of runtime to the job.

	Edgar Chen [:edgar] Reporter
	Comment 12 • 10 years ago

I am going to try MNW-OOP approach first (See comment #9).

	Edgar Chen [:edgar] Reporter
	Comment 13 • 10 years ago

Just a WIP patch.

	John Dai[:jdai]
	Comment 14 • 9 years ago

Hi Edgar,
I'd like to take this bug to follow up if you don't have concern. Thanks!

	Edgar Chen [:edgar] Reporter
	Comment 15 • 9 years ago

(In reply to John Dai[:johnz][:jdai] from comment #14)
> Hi Edgar,
> I'd like to take this bug to follow up if you don't have concern. Thanks!

Thank for taking this. :)

	John Dai[:jdai]
	Comment 18 • 9 years ago

Following the comment #9, this test doing the following things:
 1) Enabling icc tests for testing marionette webapi oop mode.
 2) Using MNW tag in treeherder to display marionette-webapi-oop result.
 3) Introducing a --test-container tag to support marionette-webapi-oop mode.

https://treeherder.allizom.org/#/jobs?repo=try&revision=82ab70b018ec


Hi James,
Is there a way let treeherder shows both marionette-webapi(MNW) and marionette-webapi-oop(MNW-OOP) tags?

	John Dai[:jdai]
	Comment 19 • 9 years ago

Treeherder log:
There has |JavascriptException: TypeError: can't convert undefined to object 
src: "const {Cc: Cc, Ci: Ci, Cr: Cr, Cu: Cu} = SpecialPowers;"| when running in MNW-OOP mode.

https://gqekosyaaaau3h4udgzbsuy4w3wc6hwey6ui2fu7scrtrfma.taskcluster-worker.net:32775/log/EVA0K3rxQvmIgduZWanmUA

	James Lal [:lightsofapollo]
	Comment 20 • 9 years ago

Hey John,

I am not quite clear on your question though it appears part of the issue may be that on TC we are not yet running MNW and on BB it's hidden see https://treeherder.allizom.org/#/jobs?repo=try&revision=82ab70b018ec&exclusion_profile=false

	John Dai[:jdai]
	Comment 21 • 9 years ago

(In reply to James Lal [:lightsofapollo] from comment #20)
> Hey John,
> 
> I am not quite clear on your question though it appears part of the issue
> may be that on TC we are not yet running MNW and on BB it's hidden see
> https://treeherder.allizom.org/#/
> jobs?repo=try&revision=82ab70b018ec&exclusion_profile=false

Hi James,

This is because I hacked job_flags.yml the marionette-webapi portion to point at b2g_emulator_marionette_webapi_oop.yml. If I don't hack job_flags.yml, it won't display marionette-webapi-oop[1]. Could you teach me how to treeherder shows both marionette-webapi(MNW) and marionette-webapi-oop(MNW-OOP) tags? Thanks.

[1] https://treeherder.allizom.org/#/jobs?repo=try&revision=e5e1e1677ce6&exclusion_profile=false

	John Dai[:jdai]
	Comment 22 • 9 years ago

I know how to add a new MNW-OOP tag, cancel needinfo request.

	John Dai[:jdai]
	Comment 23 • 9 years ago

Now marionette-webapi can support oop mode, but it always get ScriptTimeoutException. Still need to figure out why...

In this patch, marionette-webapi oop can test on local:
./mach marionette-webapi gecko/dom/cellbroadcast/tests/marionette/manifest.ini --oop

Try link:
https://treeherder.allizom.org/#/jobs?repo=try&revision=0aec801f1cfe

> 12:54:11     INFO -    File "/home/worker/build/tests/marionette/marionette/runtests.py", line 64, in <module>
> 12:54:11     INFO -      cli()
> 12:54:11     INFO -    File "/home/worker/build/tests/marionette/marionette/runtests.py", line 59, in cli
> 12:54:11     INFO -      runner = startTestRunner(runner_class, args)
> 12:54:11     INFO -    File "/home/worker/build/tests/marionette/marionette/runtests.py", line 38, in startTestRunner
> 12:54:11     INFO -      runner.run_tests(tests)
> 12:54:11     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/marionette/runner/base.py", line 799, in run_tests
> 12:54:11     INFO -      self.run_test_sets()
> 12:54:11     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/marionette/runner/base.py", line 1032, in run_test_sets
> 12:54:11     INFO -      self.run_test_set(self.tests)
> 12:54:11     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/marionette/runner/base.py", line 1011, in run_test_set
> 12:54:11     INFO -      self.run_test(test['filepath'], test['expected'], test['test_container'])
> 12:54:11     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/marionette/runner/base.py", line 981, in run_test
> 12:54:11     INFO -      self.launch_test_container()
> 12:54:11     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/marionette/runner/base.py", line 717, in launch_test_container
> 12:54:11     INFO -      }""", script_timeout=60000)
> 12:54:11     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/marionette_driver/marionette.py", line 1685, in execute_async_script
> 12:54:11     INFO -      rv = self._send_message("executeAsyncScript", body, key="value")
> 12:54:11     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/marionette_driver/decorators.py", line 36, in _
> 12:54:11     INFO -      return func(*args, **kwargs)
> 12:54:11     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/marionette_driver/marionette.py", line 716, in _send_message
> 12:54:11     INFO -      self._handle_error(resp)
> 12:54:11     INFO -    File "/home/worker/build/venv/local/lib/python2.7/site-packages/marionette_driver/marionette.py", line 770, in _handle_error
> 12:54:11     INFO -      raise errors.lookup(error)(message, stacktrace=stacktrace)
> 12:54:11     INFO -  marionette_driver.errors.ScriptTimeoutException: ScriptTimeoutException: timed out
> 12:54:11    ERROR - Return code: 1

	John Dai[:jdai]
	Comment 25 • 9 years ago

I found out the error came form "activeApp is undefined"[1] when marionette-webapi-oop launching test container.

[1] https://dxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/runner/base.py#684

> I/Gecko   (  989): 1444017662561        Marionette      DEBUG   conn7 -> {"name":"executeAsyncScript","parameters":{"scriptTimeout":60000,"sandbox":"default","debug_script":false,"script":"\nif((navigator.mozSettings == undefined) || (navigator.mozSettings == null) || (navigator.mozApps == undefined) || (navigator.mozApps == null)) {\n    marionetteScriptFinished(false);\n    return;\n}\nlet setReq = navigator.mozSettings.createLock().set({'lockscreen.enabled': false});\nsetReq.onsuccess = function() {\n    let appName = 'Test Container';\n    let activeApp = window.wrappedJSObject.Service.currentApp;\n\n    // if the Test Container is already open then do nothing\n    if(activeApp.name === appName){\n        marionetteScriptFinished(true);\n    }\n\n    let appsReq = navigator.mozApps.mgmt.getAll();\n    appsReq.onsuccess = function() {\n        let apps = appsReq.result;\n        for (let i = 0; i < apps.length; i++) {\n            let app = apps[i];\n            if (app.manifest.name === appName) {\n                app.launch();\n
> E/GeckoConsole(  989): [JavaScript Error: "TypeError: activeApp is undefined" {file: "base.py" line: 11}]
> W/SocketClient(  980): write error (Broken pipe)
> W/SocketClient(  980): write error (Broken pipe)

	David Burns :automatedtester
	Comment 26 • 9 years ago

Is this still being worked on or can I close it?

	Hsin-Yi Tsai (she/her) [:hsinyi]
	Comment 27 • 9 years ago

(In reply to David Burns :automatedtester from comment #26)
> Is this still being worked on or can I close it?

No, it's not being worked. This bug was filed for B2G RIL APIs and I assume it's no longer needed and can be closed. But to play it safer, it would be better to consult Connected Devices folks.

	David Burns :automatedtester
	Comment 28 • 8 years ago

closing as b2g related