+++ This bug was initially created as a clone of Bug #926277 +++ The patches to enable Marionette OOP test cases in bug 926277 had r+ long ago, but they still haven't made their way into m-c because of other RIL specific parts. Now they're no longer valid because of other new changes. I'm going to move them here. This bug is actually a follow-up of bug 926280.

1) create run_test_sets() and run_test_set() as basic skeleton, move all random.shuffle() calls to run_test_set(), 2) move initialization lines to run_tests() because some variables (self.device, self.capabilities) in run_test() depend on an initialized marionette instance, and we're going to move these lines out of run_test to the line right after |starttime = datetime.utcnow()|.

Move initializing line for `testargs`, `testloader` and `suite` downward to the lines right before they're going to be reference soon.

Really separate the test case enumeration part from the execution part.

1) ./mach marionette-webapi <dir>/ - selects all test cases under <dir> and execute them only in in-process mode. 2) ./mach marionette-webapi --type=+oop <dir>/ - selects all test cases under <dir> and execute them only in oop mode. 3) ./mach marionette-webapi --type=-oop <dir>/ - selects all test cases under <dir> and execute them only in in-process mode. 4) ./mach marionette-webapi <test> - execute <test> only in in-process mode. 5) ./mach marionette-webapi --type=+oop <test> - execute <test> only in oop mode. 6) ./mach marionette-webapi --type=-oop <test> - execute <test> only in in-process mode. 7) ./mach marionette-webapi <dir>/manifest.ini - selects all active test cases listed in <dir>/manifest.ini and execute them in the mode(s) according to the manifest. 8) ./mach marionette-webapi --type=+oop <dir>/manifest.ini - selects all active, oop={true,both} test cases listed in <dir>/manifest.ini and execute them only in oop mode. 9) ./mach marionette-webapi --type=-oop <dir>/manifest.ini - selects all active, oop={false,both} test cases listed in <dir>/manifest.ini and execute them only in in-process mode.

Comment on attachment 8374015 [details] [diff] [review] part 2/3: only launch test-container app when not available Previously r+ in bug 926277 comment 21 by jgriffin. Maybe you'd like to have another review?

Comment on attachment 8374016 [details] [diff] [review] part 3/3: add test cases Previously r+ in bug 926277 comment 21 by jgriffin. Maybe you'd like to have another review?

Local tries passed. https://tbpl.mozilla.org/?tree=Try&rev=5e538f4b960c

Do you mind rolling up all the client changes into one patch? I'm finding it difficult to get the big picture by looking at the full patch queue this way.

Merged patch of the obsoleted ones. Thank you :)

Comment on attachment 8375263 [details] [diff] [review] part 1/3: refine Marionette oop test case selection Review of attachment 8375263 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for all your hard work! This looks good

Comment on attachment 8374016 [details] [diff] [review] part 3/3: add test cases Review of attachment 8374016 [details] [diff] [review]: ----------------------------------------------------------------- I'm a bit confused about these tests. From what I can see, this just runs the tests with different default oop values, but doesn't do verification. How does this verify that the tests that were run were the ones we expected to run?

(In reply to Malini Das [:mdas] from comment #14) > Comment on attachment 8374016 [details] [diff] [review] > part 3/3: add test cases > > Review of attachment 8374016 [details] [diff] [review]: > ----------------------------------------------------------------- > > I'm a bit confused about these tests. From what I can see, this just runs > the tests with different default oop values, but doesn't do verification. > How does this verify that the tests that were run were the ones we expected > to run? Hi, by nature these test cases verify "those executed are in correct modes". But one thing I do miss here, as you've pointed out, is "which test cases should be executed in a specific mode". So, if that test case is not to be executed, how does it itself report that it has not been executed? This sounds like some kind of paradox. One thing we might be able to do is to allow tri-state/multi-state values in manifest. Say we can support arguments like --type=oop=[true|false|both|-|+] and move all the oop test cases selection logic into manifestparser, and then we can have test cases for it just like what we've already had in test_manifestparser.py. However this also brings another problem up to the surface -- should python test cases be able to run in oop mode since we're moving such oop feature into some more essential block? How do you think? [1]: https://mxr.mozilla.org/mozilla-central/source/testing/mozbase/manifestdestiny/tests/test_manifestparser.py

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #15) > (In reply to Malini Das [:mdas] from comment #14) > > Comment on attachment 8374016 [details] [diff] [review] > > part 3/3: add test cases > > > > Review of attachment 8374016 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > I'm a bit confused about these tests. From what I can see, this just runs > > the tests with different default oop values, but doesn't do verification. > > How does this verify that the tests that were run were the ones we expected > > to run? > > Hi, by nature these test cases verify "those executed are in correct modes". > But one thing I do miss here, as you've pointed out, is "which test cases > should be executed in a specific mode". So, if that test case is not to be > executed, how does it itself report that it has not been executed? This > sounds like some kind of paradox. > > One thing we might be able to do is to allow tri-state/multi-state values in > manifest. Say we can support arguments like > --type=oop=[true|false|both|-|+] and move all the oop test cases selection > logic into manifestparser, and then we can have test cases for it just like > what we've already had in test_manifestparser.py. However this also brings > another problem up to the surface -- should python test cases be able to run > in oop mode since we're moving such oop feature into some more essential > block? How do you think? > > [1]: > https://mxr.mozilla.org/mozilla-central/source/testing/mozbase/ > manifestdestiny/tests/test_manifestparser.py I think that manifestparser should only be concerned with parsing manifests and not ensuring that particular tests get run. In order to verify what tests get run, we should have tests against our test runner, which we currently do not have. I don't think it's within the scope of this bug to address that problem. I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=973675 to address this.

I think you're safe to land, and we'll add tests for this in bug 973675

Thank you :)

https://hg.mozilla.org/integration/b2g-inbound/rev/421c7ebcb1ed https://hg.mozilla.org/integration/b2g-inbound/rev/d86077c2ba85 https://hg.mozilla.org/integration/b2g-inbound/rev/27bdfd06723f

https://hg.mozilla.org/mozilla-central/rev/421c7ebcb1ed https://hg.mozilla.org/mozilla-central/rev/d86077c2ba85 https://hg.mozilla.org/mozilla-central/rev/27bdfd06723f