Closed Bug 1075437 Opened 6 years ago Closed 4 years ago

Support running marionette test script in both OOP and non-OOP mode

Categories

(Testing :: Marionette, defect)

x86_64
Linux
defect
Not set

Tracking

(tracking-b2g:backlog)

RESOLVED WONTFIX
tracking-b2g backlog

People

(Reporter: edgar, Unassigned)

References

Details

(Whiteboard: [affects=gaia])

Attachments

(2 files, 7 obsolete files)

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)?
Blocks: 926277
(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.
Assignee: nobody → echen
Attached patch Patch, v1 (obsolete) — Splinter Review
Attached patch Patch, v2 (obsolete) — Splinter Review
Attachment #8500010 - Attachment is obsolete: true
Attached patch Patch, v3 (obsolete) — Splinter Review
Attachment #8501535 - Attachment is obsolete: true
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.
Attachment #8503044 - Flags: review?(mdas)
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!
Attachment #8503044 - Flags: review?(mdas)
Blocks: 1032111
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.
Whiteboard: [affects=gaia]
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.
Flags: needinfo?(jgriffin)
(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.
(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!
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.
Flags: needinfo?(jgriffin)
blocking-b2g: --- → backlog
I am going to try MNW-OOP approach first (See comment #9).
blocking-b2g: backlog → ---
Attached patch WIP, Patch, v4 (obsolete) — Splinter Review
Just a WIP patch.
Attachment #8503044 - Attachment is obsolete: true
Hi Edgar,
I'd like to take this bug to follow up if you don't have concern. Thanks!
Assignee: echen → jdai
(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. :)
Attachment #8578586 - Attachment is obsolete: true
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?
Flags: needinfo?(jlal)
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
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
Flags: needinfo?(jlal)
(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
Flags: needinfo?(jlal)
I know how to add a new MNW-OOP tag, cancel needinfo request.
Flags: needinfo?(jlal)
Attached patch Support marionette-webapi-oop. (obsolete) — Splinter Review
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
Attachment #8613397 - Attachment is obsolete: true
Attachment #8613399 - Attachment is obsolete: true
Attachment #8669507 - Attachment is obsolete: true
Attachment #8669509 - Attachment description: Support marionette-webapi-oop. → Support marionette-webapi-oop.(V2)
Attached file error.log
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)
Depends on: 1211405
Is this still being worked on or can I close it?
Flags: needinfo?(jdai)
(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.
Assignee: jdai → nobody
Flags: needinfo?(jdai)
closing as b2g related
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.