Closed
Bug 952530
Opened 11 years ago
Closed 11 years ago
List all skipped/disabled tests rather than just the conditionally skipped tests
Categories
(Remote Protocol :: Marionette, defect)
Remote Protocol
Marionette
Tracking
(b2g-v1.3 fixed)
RESOLVED
FIXED
mozilla29
Tracking | Status | |
---|---|---|
b2g-v1.3 | --- | fixed |
People
(Reporter: davehunt, Assigned: davehunt)
Details
Attachments
(3 files, 2 obsolete files)
Currently we output the tests conditionally skipped in the manifest files. Ideally we would list all skipped/disabled tests along with the reason.
Assignee | ||
Comment 1•11 years ago
|
||
This patch will list all disabled tests along with the reason. If a reason is not in the manifest, but the test is conditionally skipped then the condition is the reason. Also, this will output the tests filtered out using the --type command line argument.
We should probably also include all of these tests as skipping in the results, so they appear in the HTML/XML reports. This is why I'm asking for feedback rather than review.
Comment 2•11 years ago
|
||
Comment on attachment 8350673 [details] [diff] [review]
List all skipped/disabled tests. v1.0
Review of attachment 8350673 [details] [diff] [review]:
-----------------------------------------------------------------
I agree with including the skipped tests in XML/HTML output.
Note this will likely be bitrotted by (or bitrot) bug 926277.
Attachment #8350673 -
Flags: feedback?(jgriffin) → feedback+
Assignee | ||
Comment 3•11 years ago
|
||
I can't see how we'll get the full list of skipped tests in the HTML report if we never add them to the suite. Do you have any suggestions Jonathan? Unless the answer is simple, I would suggest we take care of this in a separate bug. What do you think?
Attachment #8365900 -
Flags: review?(jgriffin)
Assignee | ||
Updated•11 years ago
|
Attachment #8350673 -
Attachment is obsolete: true
Comment 4•11 years ago
|
||
Comment on attachment 8365900 [details] [diff] [review]
List all skipped/disabled tests. v1.1
Review of attachment 8365900 [details] [diff] [review]:
-----------------------------------------------------------------
As far as including the disabled tests in HTML reports, we could simply add the disabled tests to a self.disabled_tests list, and then use that list inside HTMLReportingTestRunnerMixin.
::: testing/marionette/client/marionette/runner/base.py
@@ +818,5 @@
> + set([x['path'] for x in target_tests]))
> + for test in filtered_tests:
> + self.logger.info('TEST-SKIP | %s | filtered by type (%s)' % (
> + os.path.basename(test),
> + self.type))
So, the code starting on line 809 prints out the disabled tests; the code here prints out tests that were filtered out by --type args. But, I don't see the equivalent anywhere of the original code which prints out tests that were disabled due to device type (e.g., skipped due to being a b2g desktop build). Am I missing something?
Attachment #8365900 -
Flags: review?(jgriffin) → review-
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to Jonathan Griffin (:jgriffin) from comment #4)
> Comment on attachment 8365900 [details] [diff] [review]
> List all skipped/disabled tests. v1.1
>
> Review of attachment 8365900 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> As far as including the disabled tests in HTML reports, we could simply add
> the disabled tests to a self.disabled_tests list, and then use that list
> inside HTMLReportingTestRunnerMixin.
Sounds straight-forward enough. :)
> ::: testing/marionette/client/marionette/runner/base.py
> @@ +818,5 @@
> > + set([x['path'] for x in target_tests]))
> > + for test in filtered_tests:
> > + self.logger.info('TEST-SKIP | %s | filtered by type (%s)' % (
> > + os.path.basename(test),
> > + self.type))
>
> So, the code starting on line 809 prints out the disabled tests; the code
> here prints out tests that were filtered out by --type args. But, I don't
> see the equivalent anywhere of the original code which prints out tests that
> were disabled due to device type (e.g., skipped due to being a b2g desktop
> build). Am I missing something?
These skip-if tests are essentially conditionally disabled, so they appear in the former list. See https://github.com/mozilla/mozbase/blob/6baa74e8b71374a2279a3b478617449adcd38423/manifestdestiny/manifestparser/manifestparser.py#L1065-1069
Comment 6•11 years ago
|
||
Comment on attachment 8365900 [details] [diff] [review]
List all skipped/disabled tests. v1.1
Review of attachment 8365900 [details] [diff] [review]:
-----------------------------------------------------------------
Ah, thanks for the explanation. :)
Attachment #8365900 -
Flags: review- → review+
Assignee | ||
Comment 7•11 years ago
|
||
This patch has an issue where the 'disabled' tests are still run. I'm working on a new version, which will also hopefully incorporate the additional skipped tests in the HTML/XML reports.
Assignee | ||
Comment 8•11 years ago
|
||
Fixes the issues from the previous patch, and includes all skipped tests in the HTML/XML reports.
Attachment #8365900 -
Attachment is obsolete: true
Attachment #8366594 -
Flags: review?(jgriffin)
Assignee | ||
Comment 9•11 years ago
|
||
Example HTML report with the various skip types.
Assignee | ||
Comment 10•11 years ago
|
||
Example XML report with the various skip types.
Comment 11•11 years ago
|
||
Comment on attachment 8366594 [details] [diff] [review]
List all skipped/disabled tests. v2.0
Review of attachment 8366594 [details] [diff] [review]:
-----------------------------------------------------------------
very nice!
Attachment #8366594 -
Flags: review?(jgriffin) → review+
Comment 12•11 years ago
|
||
I'll land this for you later today if the trees re-open.
Comment 13•11 years ago
|
||
Target Milestone: --- → mozilla29
Comment 14•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
status-b2g-v1.3:
--- → fixed
Updated•2 years ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•