Remove packaging for firefox-ui-tests (and its dependency in firefox-ui-harness)

RESOLVED FIXED in Firefox 48

Status

Testing
Firefox UI Tests
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

unspecified
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

MozReview Requests

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

Attachments

(4 attachments)

(Assignee)

Description

a year ago
As of now we are packaging firefox-ui-tests to be able to access the resources folder and the manifests. We should drop those dependencies and make the tests a normal folder. With that we would also be able to run a single test via mach once bug 1237179 got implemented.

http://hg.mozilla.org/mozilla-central/file/default/testing/firefox-ui/harness/firefox_ui_harness/runners/base.py#l13

I referenced the mirrored code here because it uses the packaging for all parts, which will also land soon in our github repository.
(Assignee)

Updated

a year ago
Depends on: 1237179
(Assignee)

Comment 1

a year ago
Removing the package for tests would mean the user has to specify the path to the local test files when calling the firefox ui scripts. To ensure that this would not cause to confusion right now, I would wait with this bug until a proper documentation is in place. See bug 1237552 for details.
Depends on: 1237552
(Assignee)

Comment 2

a year ago
Thinking more about it I would wait even a bit more until we have our tests running via TaskCluster on inbound and try. If we do this now developers won't notice a difference because they use mach, but everyone else who uses the github repo would have to specify the path. I suggest to wait for bug 1237550.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
(Assignee)

Comment 3

a year ago
Created attachment 8728972 [details]
MozReview Request: Bug 1239988 - Remove dependency for firefox-ui-tests in firefox-ui-harness. r?maja_zf


This will remove any dependency for firefox-ui-tests from inside firefox-ui-harness. Affected test types are functional and update tests. To keep update tests working and testable I also added them to the mach command.

Review commit: https://reviewboard.mozilla.org/r/39205/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39205/
Attachment #8728972 - Flags: review?(mjzffr)
(Assignee)

Comment 4

a year ago
Created attachment 8728973 [details]
MozReview Request: Bug 1239988 - Remove packaging for firefox-ui-tests. r?maja_zf


The firefox-ui-tests package is no longer required. Removing it will allow us later to move tests to the appropriate places in the tree. The commit also has updates for the build system and mozharness.

Review commit: https://reviewboard.mozilla.org/r/39207/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39207/
Attachment #8728973 - Flags: review?(mjzffr)
(Assignee)

Comment 5

a year ago
Created attachment 8728974 [details]
MozReview Request: Bug 1239988 - Move resources for firefox-ui-tests out of tests folder. r?maja_zf


The resources folder should actually not be part of the tests folder. It contains test data for all tests and once tests are moved to other places the tests folder will not exist anymore.

Review commit: https://reviewboard.mozilla.org/r/39209/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39209/
Attachment #8728974 - Flags: review?(mjzffr)
(Assignee)

Comment 6

a year ago
Created attachment 8728975 [details]
MozReview Request: Bug 1239988 - Remove top-level manifest for functional and puppeteer tests. r?maja_zf


This was always a hack which made it easier for us to trigger both type of tests at once. Now with mach and mozharness we should finally get rid of it.

Review commit: https://reviewboard.mozilla.org/r/39211/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39211/
Attachment #8728975 - Flags: review?(mjzffr)
(Assignee)

Updated

a year ago
Blocks: 1237552
No longer depends on: 1237552
Comment on attachment 8728972 [details]
MozReview Request: Bug 1239988 - Remove dependency for firefox-ui-tests in firefox-ui-harness. r?maja_zf

https://reviewboard.mozilla.org/r/39205/#review35905

::: testing/firefox-ui/mach_commands.py:71
(Diff revision 1)
> -    parser.verify_usage(args)
> +    return test_types[testtype]['cli_module'].cli(args=args)

This will return 1, 10, or 0, whereas only 1 or 0 was possible in the original version. I don't know if mach has any assumptions around only 1 or 0 being possible; highlighting just in case.
Attachment #8728972 - Flags: review?(mjzffr) → review+
Comment on attachment 8728973 [details]
MozReview Request: Bug 1239988 - Remove packaging for firefox-ui-tests. r?maja_zf

https://reviewboard.mozilla.org/r/39207/#review35909

::: testing/mozharness/mozharness/mozilla/testing/firefox_ui_tests.py:215
(Diff revision 1)
> -            'abs_test_install_dir': tests_dir,
> +            'abs_test_install_dir': abs_tests_install_dir,

It seems you don't really need the abs_tests_install_dir variable.
Attachment #8728973 - Flags: review?(mjzffr) → review+
Attachment #8728974 - Flags: review?(mjzffr) → review+
Comment on attachment 8728974 [details]
MozReview Request: Bug 1239988 - Move resources for firefox-ui-tests out of tests folder. r?maja_zf

https://reviewboard.mozilla.org/r/39209/#review35913
Attachment #8728975 - Flags: review?(mjzffr) → review+
Comment on attachment 8728975 [details]
MozReview Request: Bug 1239988 - Remove top-level manifest for functional and puppeteer tests. r?maja_zf

https://reviewboard.mozilla.org/r/39211/#review35915
(Assignee)

Comment 11

a year ago
https://reviewboard.mozilla.org/r/39205/#review35905

> This will return 1, 10, or 0, whereas only 1 or 0 was possible in the original version. I don't know if mach has any assumptions around only 1 or 0 being possible; highlighting just in case.

Maybe I better revert this change then. It's clearly not related to the other changes.
(Assignee)

Comment 12

a year ago
https://reviewboard.mozilla.org/r/39207/#review35909

> It seems you don't really need the abs_tests_install_dir variable.

We need this for the requirements.txt file under `/tests/configs` below. So I cannot remove this.
(Assignee)

Comment 13

a year ago
Comment on attachment 8728972 [details]
MozReview Request: Bug 1239988 - Remove dependency for firefox-ui-tests in firefox-ui-harness. r?maja_zf

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39205/diff/1-2/
(Assignee)

Comment 14

a year ago
Comment on attachment 8728973 [details]
MozReview Request: Bug 1239988 - Remove packaging for firefox-ui-tests. r?maja_zf

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39207/diff/1-2/
(Assignee)

Comment 15

a year ago
Comment on attachment 8728974 [details]
MozReview Request: Bug 1239988 - Move resources for firefox-ui-tests out of tests folder. r?maja_zf

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39209/diff/1-2/
(Assignee)

Comment 16

a year ago
Comment on attachment 8728975 [details]
MozReview Request: Bug 1239988 - Remove top-level manifest for functional and puppeteer tests. r?maja_zf

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39211/diff/1-2/

Comment 17

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/074e0fe57a52
https://hg.mozilla.org/integration/mozilla-inbound/rev/36cd83ba0bb3
https://hg.mozilla.org/integration/mozilla-inbound/rev/76a0add3c8de
https://hg.mozilla.org/integration/mozilla-inbound/rev/1163a9d31ca6

Comment 18

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/074e0fe57a52
https://hg.mozilla.org/mozilla-central/rev/36cd83ba0bb3
https://hg.mozilla.org/mozilla-central/rev/76a0add3c8de
https://hg.mozilla.org/mozilla-central/rev/1163a9d31ca6
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.