1133752 - Convert Mozmill software-update.js library to Python/Marionette

Status: RESOLVED FIXED

(Testing :: Firefox UI Tests, defect, P1)

Description

[Bob Silverberg [:bsilverberg]]

To support the update tests, we need to convert the following library:

http://hg.mozilla.org/qa/mozmill-tests/file/e16ab8d3d62d/firefox/lib/software-update.js

Comment 1

[Bob Silverberg [:bsilverberg]]

Henrik, I've been working on this and it's going fairly well, but I am having trouble testing my code because when I run the logic that looks for the active update, I am getting None back. I assume I have to do something in my unit tests to trick the browser into thinking there is an update, but I'm not sure how to do that. Can you help?

Comment 2

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

There are two possible ways to do that. First get an older Firefox build where you definitely get an update, or try overwriting the pref app.update.url to a custom locally served XML file, which should look similar to: https://aus4.mozilla.org/update/3/Firefox/38.0a1/20150216030222/WINNT_x86_64-msvc/en-US/nightly/Windows_NT%206.1/default/default/update.xml?force=1

So something similar as we do here: https://github.com/mozilla/mozmill-automation/blob/master/mozmill_automation/testrun.py#L661

Simplest solution is an old build. :)

Comment 3

[Bob Silverberg [:bsilverberg]]

Henrik, I am still having problems retrieving an active update. I believe the code I am using to get the update, e.g., [1], is correct, but I keep getting `JavascriptException: TypeError: active_update is null`. 

I have tried three different ways of checking for updates first, including:

1. Manually checking for updates via the About window when stepping through the code with pdb.
2. Using checkForBackgroundUpdates() [2]
3. Using checkForUpdates() [3]

I also believe my calls to 2 and 3 above are working, although it was a struggle figuring out how to do them with execute_async_script.

I can keep coding some of the internals, but it's hard to test and know that what I am doing is accurate without being able to retrieve an active update.

Btw, I am using an older build, as you suggest, to ensure that there is an active update.

[1] https://gist.github.com/bobsilverberg/8e00fe467530efce7423
[2] https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsIUpdateService.idl#362
[3] https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsIUpdateService.idl#327

Comment 4

[Bob Silverberg [:bsilverberg]]

Attachment: Link to Github pull-request: https://github.com/mozilla/firefox-ui-tests/pull/90

This is still a work in progress, but I believe that enough has been done to warrant a review, so I can know how close I am to what is required. Plus, I am still having an issue with `active_update` which we can discuss when we both return on Friday.

Comment 5

[Bob Silverberg [:bsilverberg]]

I did some more work on this and found that after both checking and downloading an update, then an active update can be found. I can see how this corresponds with the usage of active_update, so that makes sense, but it makes it a bit more difficult to unit test methods that interact with an active update. For now I am either going to mock out active_update, or just not write tests for those, so I can continue with the other pieces.

Comment 6

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

Good to hear that Bob! Lets skip the tests for now, we would cover it with the real update tests for now! Also something I have seen today, you do not have to put the file manipulation (default channel pref file, allowed mar channels) stuff into the libraries. Those finally have to end-up in the harness itself. If you have already added those, lets keep them and I will move it out later.

Comment 7

[Bob Silverberg [:bsilverberg]]

Thanks Henrik. I have already added those, so you can just refactor them out later. I plan to continue today with softwareupdate and about-window.

Comment 8

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

Great to hear! Is the feedback request still wanted? Or do you have already more uptodate code to look at?

I will also start with the harness implementations early this week.

Comment 9

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8566264 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ui-tests/pull/90

I have finished converting software-update.js except for two methods: `initUpdateTests` and `assertUpdateApplied`, both of which interact with `persisted`. I'm not sure how you want to address `persisted`. I believe you suggested I leave it for last.

If you could review my work thus far that would be great, and then maybe we could have a meeting to discuss how to address `persisted`.

Comment 10

[Chris Manchester (limited bugmail, email directly)]

(In reply to Bob Silverberg [:bsilverberg] from comment #9)
> Comment on attachment 8566264 [details] [review]
> Link to Github pull-request:
> https://github.com/mozilla/firefox-ui-tests/pull/90
> 
> I have finished converting software-update.js except for two methods:
> `initUpdateTests` and `assertUpdateApplied`, both of which interact with
> `persisted`. I'm not sure how you want to address `persisted`. I believe you
> suggested I leave it for last.
> 
> If you could review my work thus far that would be great, and then maybe we
> could have a meeting to discuss how to address `persisted`.

I think the equivalent of "persisted" is bug 1121139, I can work on getting that landed.

Comment 11

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

(In reply to Bob Silverberg [:bsilverberg] from comment #9)
> If you could review my work thus far that would be great, and then maybe we
> could have a meeting to discuss how to address `persisted`.

As long as we do not have an object we can pass back to the harness, just carry an object with you in the update test itself. Later we can make sure to pass in data in setUp, and back in tearDown. As of now a missing equivalent 'persisted' feature doesn't stop our work for the tests.

Comment 12

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

Comment on attachment 8566264 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ui-tests/pull/90

Let me say this is awesome work! Lots of work happened here in the last days. Thanks a lot. So I looked at the PR and gave a lot of comments too. We may have to chat about individual things most likely on IRC to clarify them all. IMO it's not ready for a review but you get my f++!

Comment 13

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8566264 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ui-tests/pull/90

This is ready for another review.

Comment 14

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

Comment on attachment 8566264 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ui-tests/pull/90

I made a couple of comments on the PR, which need to be addressed. Please re-request for review once done. Thanks.

Comment 15

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8566264 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ui-tests/pull/90

This is ready for another review. I rebased against master, but left all the commits intact so it is easier to see which changes have just been made. They are all in the final commit.

Comment 16

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

Comment on attachment 8566264 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ui-tests/pull/90

We are close! Only a couple of things remain to be updated.

Comment 17

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

Bob, next time please select "addl. review" so that we keep the former review flags. Thanks.

Comment 18

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8566264 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ui-tests/pull/90

This is ready for another review.

Comment 19

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

Comment on attachment 8566264 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ui-tests/pull/90

This all is great work! Thanks a lot Bob! I will get it merged in a moment.

Comment 20

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

PR has been merged as https://github.com/mozilla/firefox-ui-tests/commit/2c3c1df786fe8843fb89c3c85f3eab7e8ecb73cc.

Thanks a lot Bob! This was great help from you.