Closed Bug 1133752 Opened 10 years ago Closed 10 years ago

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

Categories

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

defect

Tracking

(Not tracked)

RESOLVED FIXED
mozilla39

People

(Reporter: bsilverberg, Assigned: bsilverberg)

References

Details

Attachments

(1 file)

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
Blocks: 1133753
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?
Flags: needinfo?(hskupin)
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. :)
Flags: needinfo?(hskupin)
Priority: -- → P1
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
Flags: needinfo?(hskupin)
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.
Attachment #8566264 - Flags: feedback?(hskupin)
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.
Flags: needinfo?(hskupin)
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.
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.
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 on attachment 8566264 [details] 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`.
Attachment #8566264 - Flags: feedback?(hskupin) → review?(hskupin)
(In reply to Bob Silverberg [:bsilverberg] from comment #9) > Comment on attachment 8566264 [details] > 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.
(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.
Status: NEW → ASSIGNED
Comment on attachment 8566264 [details] 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++!
Attachment #8566264 - Flags: review?(hskupin) → feedback+
Comment on attachment 8566264 [details] Link to Github pull-request: https://github.com/mozilla/firefox-ui-tests/pull/90 This is ready for another review.
Attachment #8566264 - Flags: review?(hskupin)
Comment on attachment 8566264 [details] 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.
Attachment #8566264 - Flags: review?(hskupin) → review-
Comment on attachment 8566264 [details] 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.
Attachment #8566264 - Flags: review- → review?(hskupin)
Comment on attachment 8566264 [details] 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.
Attachment #8566264 - Flags: review?(hskupin) → review-
Bob, next time please select "addl. review" so that we keep the former review flags. Thanks.
Comment on attachment 8566264 [details] Link to Github pull-request: https://github.com/mozilla/firefox-ui-tests/pull/90 This is ready for another review.
Attachment #8566264 - Flags: review?(hskupin)
Comment on attachment 8566264 [details] 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.
Attachment #8566264 - Flags: review?(hskupin) → review+
PR has been merged as https://github.com/mozilla/firefox-ui-tests/commit/2c3c1df786fe8843fb89c3c85f3eab7e8ecb73cc. Thanks a lot Bob! This was great help from you.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Product: Mozilla QA → Testing
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: