The default bug view has changed. See this FAQ.

Move all python packages for firefox ui tests into mozilla-central

RESOLVED FIXED in Firefox 45

Status

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

People

(Reporter: maja_zf, Assigned: whimboo)

Tracking

(Blocks: 1 bug)

unspecified
mozilla46
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 fixed, firefox46 fixed)

Details

(URL)

Attachments

(4 attachments, 5 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

2 years ago
So IMO we might want to land this library under testing/marionette/firefox-puppeteer or similar, because it is hard-connected to Marionette. David, what do you think? Beside that we should make sure to create a python package from it so it can easily be used like:

from firefox_puppeteer.api.prefs import Prefs

We then can get releases out when major changes have happened and consumers outside of the tree are interested to make use of them. Problem here is definitely the branching tactic given that we will have different versions of the package.

One other idea could be to not release to PyPI but keep it only in the test package so it's bound to the build. But this can cause problems for update tests, especially when major ui changes have happened.
(Reporter)

Updated

2 years ago
Flags: needinfo?(dburns)
(Assignee)

Updated

2 years ago
Blocks: 1214372
(Assignee)

Updated

2 years ago
No longer blocks: 1214372
(Assignee)

Updated

2 years ago
Blocks: 1214378
(In reply to Henrik Skupin (:whimboo) from comment #1)
> So IMO we might want to land this library under
> testing/marionette/firefox-puppeteer or similar, because it is
> hard-connected to Marionette. David, what do you think? Beside that we
> should make sure to create a python package from it so it can easily be used
> like:


sounds good to me
Flags: needinfo?(dburns)
(Reporter)

Updated

2 years ago
Assignee: nobody → mjzffr
(Reporter)

Comment 3

a year ago
I think we can capture the branching via major.minor release numbering: e.g. 1.* is for current beta (Fx 42), 2.* is for Fx 43, and so on.
(Assignee)

Comment 4

a year ago
Personally I would like to be in sync with the version of Firefox. That means:

Firefox 44 (Nightly) -> 44.0.0
Firefox 43 (Aurora) -> 43.0.0

When some UI updates happened in Firefox whereby only some get backported to Aurora we could have:

Firefox 44 (Nightly) -> 44.1.0
Firefox 43 (Aurora) -> 43.1.0

For bug fixes we bump the patch level:

Firefox 44 (Nightly) -> 44.1.1
Firefox 43 (Aurora) -> 43.1.1

Every external consumer of puppeteer can then specify a lower and upper bound for dependencies:

setup.py:

install_requires=['firefox_puppeteer>=43.0.0, <44.0.0']

Keep in mind that we could have conflicts for update tests when the update ui is getting changed drastically between versions. But that is not a primary concern and we might be able to workaround it.
(Reporter)

Updated

a year ago
No longer blocks: 1212608
(Reporter)

Updated

a year ago
Depends on: 1217046
(Reporter)

Comment 5

a year ago
Since this no longer blocks bug 1212608, we've agreed that Henrik will take care of the move at the same time as moving firefox-ui-tests in-tree.
Assignee: mjzffr → hskupin
(Assignee)

Comment 6

a year ago
Most likely I will do it as part of my work on bug 1214372.
(Assignee)

Updated

a year ago
Depends on: 1214372
(Assignee)

Updated

a year ago
Depends on: 1232967
(Assignee)

Comment 7

a year ago
With the python packages as created on bug 1232967 we can easily move the code into m-c. So lets cover this work on this bug so that we do not pollute the tracking bug that much.
Summary: Move firefox_puppeteer in-tree → Move all python packages for firefox ui tests into mozilla-central
(Assignee)

Comment 8

a year ago
Created attachment 8699645 [details] [diff] [review]
0001-Bug-1212609-Initial-import-of-python-packages-for-fi.patch

That should be all to import our tests and packages as discussed on bug 1214372 comment 4. For now I will ask for feedback from Maja or Syd (whoever comes first) to check it code-wise. I will have to do another review round with an official peer if you both are happy with the patch.
Attachment #8699645 - Flags: feedback?(spolk)
Attachment #8699645 - Flags: feedback?(mjzffr)
(Reporter)

Updated

a year ago
Blocks: 1197234
(Reporter)

Comment 9

a year ago
Comment on attachment 8699645 [details] [diff] [review]
0001-Bug-1212609-Initial-import-of-python-packages-for-fi.patch

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

Two small changes. Everything else looks good. The directory structure makes sense to me.

::: testing/marionette/tests/firefox_ui/setup.py
@@ +13,5 @@
> +
> +setup(name='firefox_ui_tests',
> +      version=PACKAGE_VERSION,
> +      description='A collection of Mozilla Firefox UI tests run with Marionette',
> +      long_description='See https://github.com/mozilla/firefox-ui-tests',

All references to github need to be updated since that the github repo is going away.

::: testing/mozharness/mozharness/base/log.py
@@ +580,4 @@
>          for line in message.splitlines():
>              self.logger.log(self.get_logger_level(level), line)
>          if level == FATAL:
> +            print post_fatal_callback

Does this belong in this patch?
Attachment #8699645 - Flags: feedback?(mjzffr) → feedback+

Comment 10

a year ago
Comment on attachment 8699645 [details] [diff] [review]
0001-Bug-1212609-Initial-import-of-python-packages-for-fi.patch

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

I would have really liked to deep-dive comparing testing/marionette/tests/firefox_ui with the git repo https://github.com/mozilla/firefox_ui_tests. I looked through this and don't see anything alarming. I am going to go ahead and approve this.

At some point, I will do that deep compare...
Attachment #8699645 - Flags: feedback?(spolk) → feedback+
A few observations about the attached patch:

(1) Are you seeking review of the tests themselves, or just approval for moving them in-tree?

(2) Have they previously been reviewed?

(3) I have mixed feelings about bundling MarionetteTestRunner-based harnesses inside testing/marionette/harnesses and putting tests inside testing/marionette/tests.  My opinion is that we shouldn’t overload the Marionette terminology any more than we have to.

If you look at other Marionette-based tests, such as the WebAPI tests or the Loop tests, they live elsewhere in the tree.  My feeling is that tests should live close to the feature they’re testing.  Could we find a more appropriate location for the UI tests?

In my opinion testing/marionette should be reserved for the server- and client implementation.

(3) There are a few leftover debug print statements.
(Assignee)

Comment 12

a year ago
(In reply to Andreas Tolfsen (:ato) from comment #11)
> (1) Are you seeking review of the tests themselves, or just approval for
> moving them in-tree?
> 
> (2) Have they previously been reviewed?

The review request which I haven't set yet, will be for adding all the files into mozilla-central. The tests exist for a while and have all been reviewed properly.

> (3) I have mixed feelings about bundling MarionetteTestRunner-based
> harnesses inside testing/marionette/harnesses and putting tests inside
> testing/marionette/tests.  My opinion is that we shouldn’t overload the
> Marionette terminology any more than we have to.

When we talked with David he was fine with those locations. But if you think we should change something, I'm fine with it too. Maybe we can stick everything under /testing/firefoxuitest? Or any other suggestions? Updating the packaging script isn't that hard for such a move.

> If you look at other Marionette-based tests, such as the WebAPI tests or the
> Loop tests, they live elsewhere in the tree.  My feeling is that tests
> should live close to the feature they’re testing.  Could we find a more
> appropriate location for the UI tests?

Problem for us is that we have end-to-end tests which can span multiple components. So you won't find a definitive location under browser or toolkit. For some we will for sure. But we agreed on to get everything into tree first and then move the tests around to the proper places. Also before we can move the tests some more work has to be done to kill the firefox_ui_tests package, and some other awkward cross-package dependencies.

> (3) There are a few leftover debug print statements.

I assume those are the same as Maja already pointed out. I will get all that fixed.

While testing this patch I also noticed that pip installing firefox_ui_tests will leave out a lot of files. I will have to get this fixed, and maybe can directly find a way to have the same calling convention whether if our tests are getting run via tests.zip or directly via the github repository.
(Assignee)

Comment 13

a year ago
Created attachment 8700218 [details] [diff] [review]
Patch v1.1

Updated patch (not including all changes):

* Refactored firefox-ui-tests package to really install all files via pip install
* Added packaging via test_archive.py for common.tests.zip
* Added testing/config/firefox_ui_requirements.txt for mozharness script

I will fix the mentioned feedback comments on Monday, and will also talk with Andreas about the remaining open topic.
Attachment #8699645 - Attachment is obsolete: true
(Assignee)

Comment 14

a year ago
Comment on attachment 8699645 [details] [diff] [review]
0001-Bug-1212609-Initial-import-of-python-packages-for-fi.patch

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

::: testing/marionette/tests/firefox_ui/setup.py
@@ +13,5 @@
> +
> +setup(name='firefox_ui_tests',
> +      version=PACKAGE_VERSION,
> +      description='A collection of Mozilla Firefox UI tests run with Marionette',
> +      long_description='See https://github.com/mozilla/firefox-ui-tests',

I would leave this for now. At least until the github repository is still around and we have to merge changes from it into m-c. Once we run our tests from common.tests.zip and there is no need to sync anymore I will also update all references.

::: testing/mozharness/mozharness/base/log.py
@@ +580,4 @@
>          for line in message.splitlines():
>              self.logger.log(self.get_logger_level(level), line)
>          if level == FATAL:
> +            print post_fatal_callback

No, I will remove it. Thanks for spotting!
(Assignee)

Comment 15

a year ago
Created attachment 8700572 [details] [diff] [review]
Patch v1.2 (Move all fx ui tests into m-c)

Updated patch for feedback comments. Also I left the location of the files given that we have spoken with David and he agreed on it. So maybe we can have a follow-up discussion (probably public) to discuss that?
Attachment #8700218 - Attachment is obsolete: true
Attachment #8700572 - Flags: review?(ato)
(Assignee)

Comment 16

a year ago
Created attachment 8700573 [details] [diff] [review]
Patch v1 (common tests packaging changes)
Attachment #8700573 - Flags: review?(gps)
(Assignee)

Comment 17

a year ago
For the still to do mozharness changes I might want to tweak the download_and_extract step for now, which means if a test package url has been given we download and extract it. If not we will still clone the github repository for now. This would help us a lot with older branches which do not have the packages in tree, and especially for update testing release builds later cause we won't have a common.tests.zip file directly available.
(Assignee)

Comment 18

a year ago
Try push for current changes so I can use the generated test package as example for the mozharness changes: 

https://hg.mozilla.org/try/pushloghtml?changeset=1c338963a95f
(Assignee)

Comment 19

a year ago
The treeherder url might be more helpful:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c338963a95f
(Assignee)

Comment 20

a year ago
Wil, can you please give me a hint how do you create the readthedocs documentation from code which lives in tree? Thanks.
Flags: needinfo?(wlachance)
(In reply to Henrik Skupin (:whimboo) from comment #12)
> (In reply to Andreas Tolfsen (:ato) from comment #11)
> > (3) I have mixed feelings about bundling MarionetteTestRunner-based
> > harnesses inside testing/marionette/harnesses and putting tests inside
> > testing/marionette/tests.  My opinion is that we shouldn’t overload the
> > Marionette terminology any more than we have to.
> 
> When we talked with David he was fine with those locations. But if you think
> we should change something, I'm fine with it too. Maybe we can stick
> everything under /testing/firefoxuitest? Or any other suggestions? Updating
> the packaging script isn't that hard for such a move.

I think these locations would be better:

  testing/puppeteer
  testing/ui

> > If you look at other Marionette-based tests, such as the WebAPI tests or the
> > Loop tests, they live elsewhere in the tree.  My feeling is that tests
> > should live close to the feature they’re testing.  Could we find a more
> > appropriate location for the UI tests?
> 
> Problem for us is that we have end-to-end tests which can span multiple
> components. So you won't find a definitive location under browser or
> toolkit. For some we will for sure. But we agreed on to get everything into
> tree first and then move the tests around to the proper places. Also before
> we can move the tests some more work has to be done to kill the
> firefox_ui_tests package, and some other awkward cross-package dependencies.

Whilst it’s true that functional tests do tend to cover multiple different components, these tests primarily test Firefox frontend code as I understand it.

I agree moving them into tree is the highest priority, but let us in that case at least not put them in a positively wrong location:  They have nothing directly to do with the Marionette implementation, outside the fact that they are consumers of it.

We want to avoid the tight coupling that we have traditionally had between Marionette, new harnesses based on the BaseMarionetteTestRunner, their frameworks, and tests.

Admittedly there still are several things we need to look at under testing/marionette/client/* to work towards the goal of a saner dependency chain.
(Assignee)

Comment 22

a year ago
(In reply to Andreas Tolfsen (:ato) from comment #21)
>   testing/puppeteer
>   testing/ui

Ok, so ui is a bit too vague for me here. I would propose the following:

testing/puppeteer/firefox
testing/firefox-ui/harness
testing/firefox-ui/tests

Ted, would you ok with a structure like that?
Flags: needinfo?(ted)
Hopefully answered the question on irc. Short answer, look at: https://readthedocs.org/dashboard/mozbase/advanced/
Flags: needinfo?(wlachance)
(In reply to Henrik Skupin (:whimboo) from comment #22)
> Ok, so ui is a bit too vague for me here. I would propose the following:
> 
> testing/puppeteer/firefox
> testing/firefox-ui/harness
> testing/firefox-ui/tests

This is fine with me.

> Ted, would you ok with a structure like that?
Flags: needinfo?(ted)
(Assignee)

Comment 25

a year ago
Comment on attachment 8700573 [details] [diff] [review]
Patch v1 (common tests packaging changes)

It needs changes based on the latest conversation.
Attachment #8700573 - Flags: review?(gps)
(Assignee)

Comment 26

a year ago
Created attachment 8701009 [details] [diff] [review]
Patch v1.3 (Move all fx ui tests into m-c)

This moves all of our current packages to the proposed location and also includes the latest reviewed changes from bug 1232967.

Please note that our tests are currently broken if run with the latest Marionette code in tree. I would say that we figure that out in a follow-up bug and maybe Maja can work this out given that she is around next week. I will be away until Jan 3rd starting tomorrow.
Attachment #8700572 - Attachment is obsolete: true
Attachment #8700572 - Flags: review?(ato)
Attachment #8701009 - Flags: review?(ato)
(Assignee)

Comment 27

a year ago
Created attachment 8701010 [details] [diff] [review]
Patch v1.1 (common tests packaging changes)

Updated patch for new locations. Ted or gps, not sure who of you will be around but can one please review the patch?

Also I would like to know if we have to establish peers or module owners for those new sub folders. Ted, what needs to be done in those cases? Thanks.
Attachment #8700573 - Attachment is obsolete: true
Flags: needinfo?(ted)
Attachment #8701010 - Flags: review?(ted)
Attachment #8701010 - Flags: review?(gps)
(Assignee)

Comment 28

a year ago
Link to try run for the build process:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd65127cdcda&selectedJob=14813692
(Assignee)

Comment 29

a year ago
Created attachment 8701057 [details] [diff] [review]
Patch v1 (mozharness changes)

This patch changes the behavior of mozharness to the following:

* It adds support to handle the common test package for installing the harness, downloaded various binaries, and running the tests
* It still allows us to run the tests via the firefox-ui-tests github repository whereby it can be packaged or non-packaged (see bug 1232967) - so it is fully backward compatible to 38ESR
* It fixes the retrieval of the cli entry script location given that it will be different for the packaged harness (see bug 1232967 above)
* Added a new FirefoxUIFunctionalTests class to by in sync with the new cli_functional.py entry script, and keeps the base class clean.
Attachment #8701057 - Flags: review?(armenzg)
(In reply to Henrik Skupin (:whimboo)  [away 12/23 - 01/03] from comment #26)
> Please note that our tests are currently broken if run with the latest
> Marionette code in tree. I would say that we figure that out in a follow-up
> bug and maybe Maja can work this out given that she is around next week. I
> will be away until Jan 3rd starting tomorrow.

I’m happy with that since this is a Tier-3 job.
Attachment #8701009 - Flags: review?(ato) → review+
(Assignee)

Updated

a year ago
Depends on: 1234345
(Assignee)

Updated

a year ago
Component: Marionette → Firefox UI Tests
Product: Testing → Mozilla QA
QA Contact: hskupin

Comment 31

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/40fa8a5cb426
(Assignee)

Comment 32

a year ago
Comment on attachment 8701009 [details] [diff] [review]
Patch v1.3 (Move all fx ui tests into m-c)

We will bake everything over Christmas but want to get all patches backported to 45.0 because it will be our next ESR release.
Attachment #8701009 - Flags: checkin+
Comment on attachment 8701057 [details] [diff] [review]
Patch v1 (mozharness changes)

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

I won't be able to look at this before EOD and I won't be back until Jan. 4th.
Attachment #8701057 - Flags: review?(armenzg)
(Assignee)

Comment 34

a year ago
Comment on attachment 8701057 [details] [diff] [review]
Patch v1 (mozharness changes)

I asked Jordan and he would happily review it. Thanks.
Attachment #8701057 - Flags: review?(jlund)
(Assignee)

Comment 35

a year ago
(In reply to Andreas Tolfsen (:ato) from comment #30)
> > Please note that our tests are currently broken if run with the latest
> > Marionette code in tree. I would say that we figure that out in a follow-up
> > bug and maybe Maja can work this out given that she is around next week. I
> > will be away until Jan 3rd starting tomorrow.
> 
> I’m happy with that since this is a Tier-3 job.

We don't run the jobs yet with the packaged tests. First all patches on this bug have to be landed and Mozmill CI updated. So this will not happen before next year. Also when I did a test run via mozharness none of those formerly noticed failures were visible anymore. It might need more testing.

The failure i have seen when I downloaded the common.tests.zip from the above mentioned try build, extracted it, and created the venv with all the packages installed (pip install -r config/firefox_ui_requirements.txt). A call to firefox-ui-functional let Marionette open a busted chrome URL in the browser window. To test this you will also need the other two patches as attached to this bug.
Comment on attachment 8701057 [details] [diff] [review]
Patch v1 (mozharness changes)

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

looks good. couple questions before switching r- to r+

::: testing/mozharness/mozharness/mozilla/testing/firefox_ui_tests.py
@@ +135,2 @@
>              self.fatal(
>                  'Please specify --firefox-ui-branch. Valid values can be found '

do we need to update the fatal msg to include possibility that self.test_packages_url == Falsey ?

@@ +141,5 @@
>          dirs = self.query_abs_dirs()
>  
> +        # If tests are used from common.tests.zip install every Python package
> +        # via the single requirements file
> +        if self.test_packages_url:

if we fall in here _and_ we have c['virtualenv_modules'] should we warn/fatal?

@@ +216,5 @@
> +        if self.test_packages_url:
> +            super(FirefoxUITests, self).download_and_extract()
> +
> +        else:
> +            self.checkout()

you didn't have checkout() here before. why is it needed now?

@@ +292,5 @@
>          """All required steps for running the tests against an installer."""
>          dirs = self.query_abs_dirs()
>  
> +        # Import the harness to retrieve the location of the cli scripts
> +        import firefox_ui_harness

is this available in the old case (pre in-tree requirements) as well or do we need a special if/else case here like you've done above?

@@ +298,3 @@
>          cmd = [
>              self.query_python_path(),
> +            os.path.join(os.path.dirname(firefox_ui_harness.__file__),

like comment before, wondering if we need an if/else for supporting old and new way
Attachment #8701057 - Flags: review?(jlund) → review-
Comment on attachment 8701010 [details] [diff] [review]
Patch v1.1 (common tests packaging changes)

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

LGTM.
Attachment #8701010 - Flags: review?(ted)
Attachment #8701010 - Flags: review?(gps)
Attachment #8701010 - Flags: review+

Comment 38

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/40fa8a5cb426
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
(In reply to Henrik Skupin (:whimboo)  [away 12/23 - 01/03] from comment #27)
> Also I would like to know if we have to establish peers or module owners for
> those new sub folders. Ted, what needs to be done in those cases? Thanks.

The entirety of testing/ is considered one module, and I'm the module owner:
https://wiki.mozilla.org/Modules/Core#Test_Harness

If you need to add peers for this code to that list I'd be happy to help.
Flags: needinfo?(ted)
(Assignee)

Comment 40

a year ago
This bug is not fixed! It still needs two patches to be landed.
Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---
(Assignee)

Updated

a year ago
Attachment #8701010 - Flags: checkin+

Comment 41

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/642a536b741a
(Assignee)

Comment 42

a year ago
Comment on attachment 8701057 [details] [diff] [review]
Patch v1 (mozharness changes)

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

::: testing/mozharness/mozharness/mozilla/testing/firefox_ui_tests.py
@@ +135,2 @@
>              self.fatal(
>                  'Please specify --firefox-ui-branch. Valid values can be found '

Would make sense. Maybe I separate those two checks into their own if conditions.

@@ +141,5 @@
>          dirs = self.query_abs_dirs()
>  
> +        # If tests are used from common.tests.zip install every Python package
> +        # via the single requirements file
> +        if self.test_packages_url:

If any other virtualenv modules have been specified, those won't be overwritten, right? We simply extend the array for all modules as listed in the firefox_ui_requirements.txt file.

@@ +216,5 @@
> +        if self.test_packages_url:
> +            super(FirefoxUITests, self).download_and_extract()
> +
> +        else:
> +            self.checkout()

`checkout()` was a specific step. Given that we don't need it as such in the future, I already removed it here. See the change in line 111 of this file.

@@ +292,5 @@
>          """All required steps for running the tests against an installer."""
>          dirs = self.query_abs_dirs()
>  
> +        # Import the harness to retrieve the location of the cli scripts
> +        import firefox_ui_harness

Yes, it's not its own package but an importable module:

https://github.com/mozilla/firefox-ui-tests/tree/mozilla-central/firefox_ui_harness

FYI the Github repo will also get those individual packages soon for the mozilla-central and mozilla-aurora branch. Right now it's part of the python packages branch:

https://github.com/mozilla/firefox-ui-tests/tree/python_packages

All those three different paths will be compatible to each other given that I want to have the backport compatibility down to esr38.

@@ +298,3 @@
>          cmd = [
>              self.query_python_path(),
> +            os.path.join(os.path.dirname(firefox_ui_harness.__file__),

Nope, we don't need.
(Assignee)

Updated

a year ago
Flags: needinfo?(jlund)
(Assignee)

Comment 43

a year ago
Created attachment 8703831 [details] [diff] [review]
Patch v2 (mozharness changes)

While checking the mozharness patch I found two other cases which were missing! So here all the changes:

* We also have to check for test_url and not only test_packages_url. Appropriately the if conditions need to be updated

* There was still the @PreScriptAction hook for the former `checkout` around. I removed it now given that we don't need it.

I left out any changes to the remaining open question about the virtualenv modules because I think that this is not necessary and we simply extend the array.
Attachment #8701057 - Attachment is obsolete: true
Flags: needinfo?(jlund)
Attachment #8703831 - Flags: review?(jlund)
(Assignee)

Comment 44

a year ago
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #39)
> The entirety of testing/ is considered one module, and I'm the module owner:
> https://wiki.mozilla.org/Modules/Core#Test_Harness
> 
> If you need to add peers for this code to that list I'd be happy to help.

If it's only about adding names (in this case Maja and Syd) to this list and updating the components to the firefox_ui tests, I can do that on my own. Or is there anything else to do?
Flags: needinfo?(ted)
(Assignee)

Comment 45

a year ago
Given that Firefox 45 will become our next ESR release, we also want to get those patches backported to mozilla-aurora. I will do those backports once the remaining patch has been landed and everything works correctly.
status-firefox45: --- → affected
(Assignee)

Updated

a year ago
No longer blocks: 1214378
Depends on: 1214378

Comment 46

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/642a536b741a
(Assignee)

Comment 47

a year ago
Here some example jobs via mozmill-ci and the patched mozharness code:

1. With the packed tests archive:
https://treeherder.allizom.org/logviewer.html#?job_id=2623823&repo=mozilla-central

2. Packaged tests from Github:
https://treeherder.allizom.org/logviewer.html#?job_id=2623899&repo=mozilla-central

3. Non-packaged tests from Github:
https://treeherder.allizom.org/logviewer.html#?job_id=2623898&repo=mozilla-central
Comment on attachment 8703831 [details] [diff] [review]
Patch v2 (mozharness changes)

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

lgtm
Attachment #8703831 - Flags: review?(jlund) → review+
(Assignee)

Updated

a year ago
Attachment #8703831 - Flags: checkin+

Comment 49

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/97ab593aff2b
(Assignee)

Updated

a year ago
Blocks: 1237179
(Assignee)

Updated

a year ago
Keywords: leave-open

Comment 50

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/97ab593aff2b
Status: REOPENED → RESOLVED
Last Resolved: a year agoa year ago
Resolution: --- → FIXED
Product: Mozilla QA → Testing
(Assignee)

Updated

a year ago
Depends on: 1237322
(Reporter)

Updated

a year ago
Blocks: 1237760
(Assignee)

Comment 51

a year ago
We cannot simply take the attached patch for firefox-ui-tests for mozilla-aurora. There are a lot of differences. So we should mirror the code from our github repositories mozilla-aurora branch.

To track that its better for me to leave this bug open until it has been done.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: --- → mozilla46
(In reply to Henrik Skupin (:whimboo) from comment #44)
> If it's only about adding names (in this case Maja and Syd) to this list and
> updating the components to the firefox_ui tests, I can do that on my own. Or
> is there anything else to do?

Sorry about that:
https://wiki.mozilla.org/Modules/Core#Test_Harness
Flags: needinfo?(ted)
(Assignee)

Comment 53

a year ago
Created attachment 8709615 [details] [diff] [review]
Patch v1 (Move all fx ui tests into mozilla-aurora)

This is the specific backport of our harness, puppeteer, and tests for mozilla-aurora. We cannot simply backport the existing patch due to various differences.

For the packaging patch I will request uplift to m-a tomorrow.
Attachment #8709615 - Flags: review?(mjzffr)
(Reporter)

Updated

a year ago
Attachment #8709615 - Flags: review?(mjzffr) → review+
(Assignee)

Comment 54

a year ago
try job with the packaging patch included:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9acc6833bb2

Something I haven't tested is how the Marionette releases on mozilla-aurora would work with our tests. Maybe we have to downgrade it to the appropriate versions to not run different releases in mozmill-ci for in-tree and external tests. But lets see.
(Assignee)

Comment 55

a year ago
(In reply to Henrik Skupin (:whimboo) from comment #54)
> try job with the packaging patch included:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9acc6833bb2

New try run because for testing I missed to add the firefox_ui_requirements.txt file:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c359ac5b8a8b

> Something I haven't tested is how the Marionette releases on mozilla-aurora
> would work with our tests. Maybe we have to downgrade it to the appropriate
> versions to not run different releases in mozmill-ci for in-tree and
> external tests. But lets see.

This is actually not a problem given that on aurora we exactly have client==2.0.0 and driver=1.1.1. So it should work without any issues.
(Assignee)

Comment 56

a year ago
The try builds are passing and packaging of our tests works fine. So I'm going to land both patches on mozilla-aurora with a=testonly, as agreed with Tomcat earlier today.
Status: REOPENED → ASSIGNED
(Assignee)

Comment 57

a year ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/4a091c582260
https://hg.mozilla.org/releases/mozilla-aurora/rev/cf7672d66354

We do not need the mozharness changes given that we always use mozharness from mozilla-central.

So this is finally done. Thanks all for your help and feedback.
Status: ASSIGNED → RESOLVED
Last Resolved: a year agoa year ago
status-firefox45: affected → fixed
Resolution: --- → FIXED
(Assignee)

Comment 58

a year ago
Currently we run our tests in mozmill-ci via the mozilla-central version of mozharness. But this might change for the 45ESR branch. I don't want to keep backward-compat code around for more than a year. So lets get the mozharness changes uplifted for 45:

https://hg.mozilla.org/releases/mozilla-aurora/rev/b9cf5e85cf7f
You need to log in before you can comment on or make changes to this bug.