1287591 - The application should not be started and left open if evaluation of manifest files do not find any test

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

If you run one of the marionette test commands via mach, and no tests can be found AFTER evaluating the manifest files for eg. tags, we spawn the application but do not close it anymore. Here an example:

mach firefox-ui-update --binary /mozilla/bin/nightly/firefox testing/firefox-ui/tests/functional/keyboard_shortcuts/manifest.ini

For the update tests we are looking for a specific tag in the manifest which is not present for functional tests. As result Firefox gets opened and stays open forever. We should really stop the instance if we launched it ourselves.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1287591 - The application should not be started and left open if filtering of a single manifest file does not find any test

Review commit: https://reviewboard.mozilla.org/r/67458/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67458/

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Anjana, we decided to land this without a harness unit test. If you think that one should be added, would you mind doing it? The pre-condition is an empty manifest file you specify for the tests. Thanks.

Comment 3

[David Burns :automatedtester]

Comment on attachment 8775269 [details]
Bug 1287591 - The application should not be started and left open if filtering of a single manifest file does not find any test

https://reviewboard.mozilla.org/r/67458/#review65184

Comment 4

[Anjana Vakil (:vakila)]

(In reply to Henrik Skupin (:whimboo) from comment #2)
> Anjana, we decided to land this without a harness unit test. If you think
> that one should be added, would you mind doing it? The pre-condition is an
> empty manifest file you specify for the tests. Thanks.

Sure - I'm adding a test to make sure that `self.marionette` and `self.httpd` get reset to `None` after trying to call `run_tests` with a manifest with no active tests. For me, the test fails before applying the patch you submitted for this bug, and passes after applying it.  Let me know what you think!

Comment 5

[Anjana Vakil (:vakila)]

Attachment: Bug 1287591 - Test cleanup after adding empty manifest;

Add a test to make sure that if an empty manifest, or one with no active tests,
is provided, Marionette/Firefox are shut down as expected (previous behavior
was that the Marionette cleanup was never reached in this scenario).

Review commit: https://reviewboard.mozilla.org/r/68308/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68308/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c77f60f1fef9
The application should not be started and left open if filtering of a single manifest file does not find any test r=automatedtester

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c77f60f1fef9