Provide a way to do data-driven tests with Marionette test runner

RESOLVED FIXED in mozilla35

Status

Testing
Marionette
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: zac, Assigned: parkouss)

Tracking

unspecified
mozilla35
Other
All
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [affects=b2g])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

3 years ago
It would be great to have a nice clean way to do data-driven tests in the Marionette test runner.


ddt has a nice implementation but it does not accept a variable unfortunately:
http://ddt.readthedocs.org/en/latest/example.html
Whiteboard: [affects=webqa]
(Assignee)

Comment 1

3 years ago
I think I wrote something similar for one project recently:

- http://funq.readthedocs.org/en/latest/user_api/testcase.html#funq.testcase.parameterized
- http://funq.readthedocs.org/en/latest/user_api/testcase.html#funq.testcase.with_parameters

It is pretty close to what ddt do as far as I understand - what do you mean Zac with "it does not accept a variable unfortunately" ? It seems to me that both ddt and the other implementation accepts variables to dynamically create new methods. Can you give me some examples of what you would like to see ?
Flags: needinfo?(zcampbell)
(Reporter)

Comment 2

3 years ago
Hey Julien, that looks really good, I wish I'd found this when I originally googled around! 
Yes I'd like to be able to pass in a Python list, for example, and have that become the test data.

I'll have to speak to the maintainers (eg mdas) to see how it could integrate into our existing framework.
Flags: needinfo?(zcampbell) → needinfo?(mdas)
(In reply to Zac C (:zac) from comment #2)
> Hey Julien, that looks really good, I wish I'd found this when I originally
> googled around! 
> Yes I'd like to be able to pass in a Python list, for example, and have that
> become the test data.
> 
> I'll have to speak to the maintainers (eg mdas) to see how it could
> integrate into our existing framework.

Looks like we just need to change the base class that CommonTestCase inherits from to be funq.testcase.FunqTestCase instead of the unittest's basic TestCase class, and then you can import the decorators and use them. I haven't tested it, but it seems like a simple change.
Flags: needinfo?(mdas)
(Assignee)

Comment 4

3 years ago
In fact I think it may be easier to only add a __metaclass__ attribute on CommonTestCase (with a value of MetaParameterized) and copy paste the implementation of MetaParameterized, parameterized and with_parameters from funq. Then import decorators and use them will just work, but you won't require the funq package that I think is not relevant here.

The needed implementation only takes a few lines, as you can see here: http://funq.readthedocs.org/en/latest/_modules/funq/testcase.html#with_parameters. Plus you can adapt if there is something you don't like.

I can do the work, and attach a patch, if you're interested.
Flags: needinfo?(mdas)
(Reporter)

Updated

3 years ago
Whiteboard: [affects=webqa] → [affects=b2g]
(In reply to Julien Pagès from comment #4)
> In fact I think it may be easier to only add a __metaclass__ attribute on
> CommonTestCase (with a value of MetaParameterized) and copy paste the
> implementation of MetaParameterized, parameterized and with_parameters from
> funq. Then import decorators and use them will just work, but you won't
> require the funq package that I think is not relevant here.
> 
> The needed implementation only takes a few lines, as you can see here:
> http://funq.readthedocs.org/en/latest/_modules/funq/testcase.
> html#with_parameters. Plus you can adapt if there is something you don't
> like.
> 
> I can do the work, and attach a patch, if you're interested.

Sure, that sounds great, thanks!
Flags: needinfo?(mdas)
(Assignee)

Updated

3 years ago
Assignee: nobody → j.parkouss
(Assignee)

Comment 6

3 years ago
Created attachment 8479765 [details] [diff] [review]
Provide a way to do data-driven tests with Marionette test runner

I modified the test_simpletest_sanity.py file in the same file to demonstrate how to use the @parameterized decorator - tell me if it is not relevant. :)
Attachment #8479765 - Flags: review?(mdas)
Comment on attachment 8479765 [details] [diff] [review]
Provide a way to do data-driven tests with Marionette test runner

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

works well! can you remove the extra whitespace from marionette_test.py? They show up in red if you look at that file on this page: https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1047101&attachment=8479765.

r- for now because I'd like a unit test for this decorator. Adding it to test_simpletest_sanity.py doesn't make much sense as a unit-test, and the other issue here is that test_simpletest_sanity.py should reset the context in a tearDown function in case anything fails during the test.
Attachment #8479765 - Flags: review?(mdas) → review-
(Assignee)

Comment 8

3 years ago
Created attachment 8480451 [details] [diff] [review]
Provide a way to do data-driven tests with Marionette test runner

I removed the blank lines and the modifications done to test_simpletest_sanity.py - it was a bad idea - and wrote some unit tests instead as you recommended. :)
Attachment #8479765 - Attachment is obsolete: true
Attachment #8480451 - Flags: review?(mdas)
Comment on attachment 8480451 [details] [diff] [review]
Provide a way to do data-driven tests with Marionette test runner

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

Nice, the test looks great! But there's one more thing missing: the new test needs to be added to https://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/tests/unit/unit-tests.ini so it can be run on tbpl. Can you update your patch to do this? I'll r+ then, thanks!

Oh, and if you get around to this after today, can you ask for r? from either jgriffin or AutomatedTester? I'll be gone for a week.
Attachment #8480451 - Flags: review?(mdas)
(Assignee)

Comment 10

3 years ago
Created attachment 8481813 [details] [diff] [review]
Provide a way to do data-driven tests with Marionette test runner

:mdas, I added the new test to the tests/unit/unit-tests.ini file. :jgriffin, you are now the final reviewer for this patch since :mdas won't be here fo a week (see comment #9) - thanks !
Attachment #8480451 - Attachment is obsolete: true
Attachment #8481813 - Flags: review?(jgriffin)
Comment on attachment 8481813 [details] [diff] [review]
Provide a way to do data-driven tests with Marionette test runner

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

Nice!  Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=997e029ff7d5
Attachment #8481813 - Flags: review?(jgriffin) → review+
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/9906ac54c8fc
Flags: in-testsuite+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9906ac54c8fc
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.