Bug 1129843 - Convert update tests to Marionette #122
Conversation
| @@ -2,4 +2,7 @@ | |||
| # License, v. 2.0. If a copy of the MPL was not distributed with this | |||
| # file, You can obtain one at http://mozilla.org/MPL/2.0/. | |||
|
|
|||
| from runtests import cli | |||
| from mixins import * | |||
There was a problem hiding this comment.
I'm not a fan of wildcard imports (or mixins), but I'll keep reading.
There was a problem hiding this comment.
Mixins no longer exist. I was using them at the beginning because it was easier to get it working by mostly copying from the gaia tests. But with different entry points it was a nightmare. I will finally update the folder when all is ready.
whimboo
changed the title
|
|
||
| :param flags: Specific restart flags for Firefox | ||
| """ | ||
| self.marionette.restart() |
There was a problem hiding this comment.
Don't we need the in application initiated restart? Wasn't this a hard and urgent requirement for implementing these tests?
There was a problem hiding this comment.
I had to remove it at some point because it was not working. I wanted to investigate it but looks like I forgot about it. I tried to re-add in_app=True but the tests are still failing because Firefox doesn't restart! I will get this investigated and a bug filed. Thanks for noticing that!
There was a problem hiding this comment.
Looks like the locally installed packages for driver and client were totally messed-up. When installing the client from m-c it downloads the driver from pypi but not picking it up from m-c too. I fixed that and it works now.
|
|
||
| finally: | ||
| self.updates[self.current_update_index]['patch'] = about_window.patch_info | ||
|
|
There was a problem hiding this comment.
This method from here up is shared with the other class.
There was a problem hiding this comment.
I can combine that for sure. Just wanted to see what could be necessary for multiple update steps. But once we work on those we can split the shared code where necessary.
| # Dictionary which holds the information for each update | ||
| self.updates = [{ | ||
| 'build_pre': self.software_update.build_info, | ||
| 'build_post': self.software_update.build_info, |
There was a problem hiding this comment.
Let's not add this key until after the update is performed. I had to look at this pretty closely to conclude it does the right thing based on build_post getting updated after the update. The assertions are mostly using assertEqual, so if we had mistakenly been aliasing build_pre in build_post they would almost all pass despite the error.
There was a problem hiding this comment.
We have to do this to workaround the fact that a possible crash or whatever else doesn't let us finish the test. If we move this assignment to the end of the test, we won't have a post build assigned. This would add a lot more extra cases to the dashboard (treeherder view).
Even that we are safer now for Python tests, I would propose that we keep this for now and if necessary find an alternative solution as a follow-up bug. The code as is has been proven to work over the last years.
There was a problem hiding this comment.
Then can you have its value be None here? I'm skeptical of making decisions here based on a dashboard we haven't made yet.
There was a problem hiding this comment.
I will do it. So we will tweak it when I returned.
Currently a WIP which will get frequent updates over the next days. Nothing to review yet.