+++ This bug was initially created as a clone of Bug #1217046 +++ In bug 1217046 we moved the testcase classes from the firefox ui harness to puppeteer and firefox-ui-tests. While working to get all the pieces into mozilla-central I noticed some problems: 1. The puppeteer package does only need the FirefoxTestCase class for the unit tests, but not otherwise. This should not qualify to have the testcase class in that package. 2. The UpdateTestCase class is even in the firefox-ui-tests sub folder. This folder will no longer be present when everything is in tree. Tests will be located close to the integration code. All in all the best place would still be the harness package to host the testcase classes. We don't expect that someone who is using our entry scripts will use a plain Marionette testcase but always our puppeteer package to access UI and API classes. So the harness has to provide those classes. The changeset as landed on bug 1217046 might not be easily reverted due to changes happened in the meantime. So a new patch might be necessary.

So we can revert the changeset 50527c612f3c063ab52046d0d01f610b64f5f003 even now on mozilla-central. But a small follow-up patch is necessary for the test_enable_privilege.py test.

firefox-media-tests (and potentially other test suites) need FirefoxTestCase, which is one of the reasons behind moving FirefoxTestCase into Puppeteer: it makes more sense for a suite like media-tests to depend on Puppeteer then on firefox-ui-harness. Based on my experience with media-tests, I think new test suites will not need anything else in firefox_ui_harness, and any harness code that is common to all test suites should live in MarionetteHarness anyway. I propose we keep FirefoxTestCase in puppeteer, and suite-specific test cases (like MediaTestCase, UpdateTestCase) in suite-specific harness packages.

Discussion 02/12/15 (AutomatedTester) I wasn't understanding what you were mentioning earlier... 22:08 (AutomatedTester) why does FirefoxTestCase live in puppeteer? 22:09 (AutomatedTester) to make it useable by other "firefox" based tests? 22:09 (whimboo) easier usable 22:09 (whimboo) without having to require firefox-ui-harness 22:10 (whimboo) my feeling is that testcase classes should be close to the harness 22:11 (AutomatedTester) yea... 22:11 (AutomatedTester) so my guy says "puppeteer" should be equivalent to web page objects 22:11 (AutomatedTester) a DSL to help write tests 22:12 (AutomatedTester) and anything testcase related should be in the harness 22:12 (AutomatedTester) if you are writing tests you need to build on a harness 22:12 (AutomatedTester) and to get to specific parts of the AUT you need to use the DSL 22:12 (AutomatedTester) but if you just want to drive the AUT to a specific thing then you only need the DSL 22:13 (AutomatedTester) that's how I would split them 22:14 (sydpolk) what is "DSL" and "AUT"? 22:14 (maja_zf) domain specific language? 22:14 (maja_zf) i dunno AUT 22:15 (AutomatedTester) application under test 22:15 (AutomatedTester) domain specific language 22:15 (whimboo) AutomatedTester: so a DSL is optional or fixed set and needs to be used? 22:15 (whimboo) i asked because we currently have the latter situation 22:15 (whimboo) class FirefoxTestCase(MarionetteTestCase, Puppeteer): 22:16 (AutomatedTester) let's get a concrete example of what I mean 22:16 (AutomatedTester) https://github.com/mozilla/Addon-Tests/ 22:17 (AutomatedTester) the pages dir is what I see as puppeteer 22:17 (maja_zf) AutomatedTester: ok, but FirefoxTestCase is useful to many test suites, so that's why I was thinking in should not live in firefox-ui-harness. perhaps it should be in marionette_test.py for common use? but then marionette client has puppeteer as a dependency -- I'm not sure we want that. 22:18 (AutomatedTester) maja_zf: why does it need to have a reference to puppeteer? 22:18 mwobensmith has left IRC (Quit: Leaving.) 22:18 (whimboo) AutomatedTester: see my above line 22:18 (maja_zf) "FirefoxTestCase(MarionetteTestCase, Puppeteer)" 22:18 (whimboo) it has been setup that way by ahal or chmanchester initially 22:18 (AutomatedTester) that doesnt answer my question 22:18 (AutomatedTester) why "FirefoxTestCase(MarionetteTestCase, Puppeteer)" 22:18 (AutomatedTester) why not "FirefoxTestCase(MarionetteTestCase)" 22:19 (AutomatedTester) can we separate concerns 22:19 (whimboo) that is what i'm trying to solve now. if that would help to get rid of the location question, we would be closer to a solution 22:20 (whimboo) so that puppeteer could optionally be used 22:20 (whimboo) but is not required 22:20 (AutomatedTester) I dont see why location matters for my question 22:21 (whimboo) so FirefoxTestCase basically sets up some properties like self.browser which is provided via puppeteer 22:21 (maja_zf) AutomatedTester: puppeteer is used by FirefoxTestCase setup and teardown, managing window focus/prefs. Also seems to be handy for helper methods like restart 22:22 (whimboo) AutomatedTester: https://github.com/mozilla/firefox-ui-tests/blob/mozilla-central/firefox_puppeteer/testcases/base.py

My suggestion is that we should separare concerns here, which may/may not be a lot of work. I feel like we should remove any ties between harness and puppeteer so that we can move things and grow them independently. Puppeteer can have a dependency on marionette_driver and only be concerned in getting things to the place where we expect it in Firefox. This should be nothing more than a DSL (Domain Specific Language) e.g. https://github.com/mozilla/Addon-Tests/blob/master/pages/desktop/addons_site.py The harness is there to do the reporting and test discovery. We can extend it to starting the browser but personally I think the "tests" should be as self contained as possible. My ideal (and this might not be entirely possible currently) is to have tests looking like https://github.com/mozilla/Addon-Tests/blob/master/tests/desktop/test_reviews.py Having things separate means we can try update things independently if need be and can try limit who would need to do updates. HTH

So we spoke on IRC and for now we will move the UpdateTestCase also to the puppeteer package. Just to have them located right next to each other. A more in-depth refactoring as David pointed out above will happen later.

Pushed to mozilla-central: https://github.com/mozilla/firefox-ui-tests/commit/823a9f0f3cf50093a54cc73cc6d8e27bc97f52f8 I will care of other branches on Monday.

I backported the changes to beta and release but left esr38 out given that it has merge issues and I do not see a value in having the changes there. https://github.com/mozilla/firefox-ui-tests/commit/9dab39b2ee8122def004608ed496780b10e37cba (beta) https://github.com/mozilla/firefox-ui-tests/commit/0f7cd5b2c5b5b8bda24de63a6c4922057b7fcb90 (release)