Separate Marionette harness unit tests into class-specific modules

RESOLVED FIXED in Firefox 51

Status

Testing
Marionette
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: vakila, Assigned: vakila)

Tracking

(Blocks: 1 bug, {ateam-marionette-harness-tests})

Version 3
mozilla51
ateam-marionette-harness-tests
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

a year ago
At the moment, harness unit tests for various runner classes (from [1],[2]) are all contained in a single module, `tests/harness_unit/test_marionette_runner.py`[3]. 

It would be better to divide this into separate modules that are specific to a certain class (or perhaps a related group of classes), e.g. `test_marionette_runner.py`, `test_marionette_arguments.py`, `test_marionette_test_result.py` etc. 


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

[2] https://dxr.mozilla.org/mozilla-central/source/testing/marionette/harness/marionette/runtests.py

[3] https://dxr.mozilla.org/mozilla-central/source/testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py
(Assignee)

Updated

a year ago
Depends on: 1292300
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Assignee: nobody → anjanavakil

Comment 3

a year ago
mozreview-review
Comment on attachment 8785294 [details]
Bug 1291796 - Split Marionette harness tests by class;

https://reviewboard.mozilla.org/r/74550/#review72522
Attachment #8785294 - Flags: review?(mjzffr) → review+

Comment 4

a year ago
mozreview-review
Comment on attachment 8785295 [details]
Bug 1291796 - Minor fixes to harness unit tests;

https://reviewboard.mozilla.org/r/74552/#review72524

::: testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py:81
(Diff revision 1)
>  def manifest_fixture(request):
>      '''
>      Fixture for the contents of mock_manifest, where a manifest
>      can include enabled tests, disabled tests, both, or neither (empty)
>      '''
> +    from manifestparser import TestManifest

Since this is parametrized, lets avoid running the import over and over. If you just import manifestparser once at the top and then refer to manifestparser.TestManifest in the test(s), the pytest warning should go away.
Attachment #8785295 - Flags: review?(mjzffr) → review-
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 7

a year ago
mozreview-review
Comment on attachment 8785295 [details]
Bug 1291796 - Minor fixes to harness unit tests;

https://reviewboard.mozilla.org/r/74552/#review72772

Thanks!
Attachment #8785295 - Flags: review?(mjzffr) → review+

Comment 8

a year ago
Pushed by mjzffr@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6a81df9a2129
Split Marionette harness tests by class; r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/0d7beb4dc862
Minor fixes to harness unit tests; r=maja_zf
(Assignee)

Updated

a year ago
Blocks: 1298900

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6a81df9a2129
https://hg.mozilla.org/mozilla-central/rev/0d7beb4dc862
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Keywords: ateam-marionette-harness-tests
You need to log in before you can comment on or make changes to this bug.