Merge lp:~ahayzen/webbrowser-app/fix-1491279-depend-on-download-manager into lp:webbrowser-app/staging

Proposed by Andrew Hayzen
Status: Merged
Merged at revision: 1575
Proposed branch: lp:~ahayzen/webbrowser-app/fix-1491279-depend-on-download-manager
Merge into: lp:webbrowser-app/staging
Diff against target: 90 lines (+31/-10)
2 files modified
debian/control (+5/-0)
tests/autopilot/webbrowser_app/tests/test_downloads.py (+26/-10)
To merge this branch: bzr merge lp:~ahayzen/webbrowser-app/fix-1491279-depend-on-download-manager
Reviewer Review Type Date Requested Status
Olivier Tilloy Approve
Review via email: mp+309669@code.launchpad.net

Commit message

* Add qtdeclarative5-ubuntu-content1 and qtdeclarative5-ubuntu-download-manager0.1 as depends as they are in main and then fixes issues with saving images
* Enable autopilot TestDownloads on desktop as depends are now met

Description of the change

* Add qtdeclarative5-ubuntu-content1 and qtdeclarative5-ubuntu-download-manager0.1 as depends as they are in main and then fixes issues with saving images
* Enable autopilot TestDownloads on desktop as depends are now met

To post a comment you must log in.
Revision history for this message
Olivier Tilloy (osomon) wrote :

Please keep the list of dependencies alphabetically sorted.

Please add those two runtime deps to webapp-container as well. The fact that shared QML files are being installed by the webbrowser-app package is an implementation detail that might not hold true forever, so having the dependencies list explicitly is more future-proof.

review: Needs Fixing
1565. By Andrew Hayzen

* Fixed alphabetical ordering
* Added depends to webapp-container

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

Fixed the alphabetical ordering and added webapp-container deps, please retest :-)

1566. By Andrew Hayzen

* Enable TestDownloads on desktop as depends are now met

Revision history for this message
Olivier Tilloy (osomon) wrote :

webbrowser_app.tests.test_downloads.TestDownloads.test_picker reliably fails on my desktop, most likely because there’s no app installed that can handle the application/pdf mimetype.
Ideally this should be mocked in contenthub so that the test can always be run with predictable results. If not possible, please make it conditional (or remove it altogether, are we guaranteed to have a pdf reader application always installed on touch devices?).

review: Needs Fixing
1567. By Andrew Hayzen

* Run content-hub-peer-hook-wrapper in the setup for the test_picker test, to ensure that test peers are registered
* Add content-hub-testability as a depends for webbrowser-app autopilot

1568. By Andrew Hayzen

* Merge of lp:webbrowser-app/staging

1569. By Andrew Hayzen

* Reduce diff

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

Updated this to use content-hub-testability (note you'll need to install the package). And to ensure that the test peer has been registered before the test, in the setUp it runs /usr/lib/*/content-hub/content-hub-peer-hook-wrapper.

Please retest :-)

1570. By Andrew Hayzen

* Use UITK autopilot helper to get the arch triplet for finding content-hub-peer-hook-wrapper

Revision history for this message
Olivier Tilloy (osomon) wrote :

It looks like subprocess.run(…) is a recent addition to the subprocess module, it’s not available on my arale’s python3:

phablet@ubuntu-phablet:~$ python3 -c "import subprocess; subprocess.run"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
AttributeError: 'module' object has no attribute 'run'

review: Needs Fixing
1571. By Andrew Hayzen

* Use check_call instead of run from subprocess as this was added in Python 3.5 and isn't on devices

Revision history for this message
Andrew Hayzen (ahayzen) wrote :

Updated to use check_call, please retest :-)

Revision history for this message
Olivier Tilloy (osomon) wrote :

LGTM now, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/control'
--- debian/control 2016-10-27 08:21:44 +0000
+++ debian/control 2016-11-03 18:00:54 +0000
@@ -53,6 +53,8 @@
53 qml-module-qtquick-layouts,53 qml-module-qtquick-layouts,
54 qml-module-qtquick-window2 (>= 5.3),54 qml-module-qtquick-window2 (>= 5.3),
55 qml-module-ubuntu-web (= ${binary:Version}),55 qml-module-ubuntu-web (= ${binary:Version}),
56 qtdeclarative5-ubuntu-content1,
57 qtdeclarative5-ubuntu-download-manager0.1,
56 qtdeclarative5-ubuntu-ui-toolkit-plugin (>= 1.3) | qtdeclarative5-ubuntu-ui-toolkit-plugin-gles (>= 1.3),58 qtdeclarative5-ubuntu-ui-toolkit-plugin (>= 1.3) | qtdeclarative5-ubuntu-ui-toolkit-plugin-gles (>= 1.3),
57 qtdeclarative5-unity-action-plugin,59 qtdeclarative5-unity-action-plugin,
58Replaces: webbrowser-app-assets,60Replaces: webbrowser-app-assets,
@@ -76,6 +78,8 @@
76 qml-module-qtquick-window2 (>= 5.3),78 qml-module-qtquick-window2 (>= 5.3),
77 qml-module-ubuntu-onlineaccounts,79 qml-module-ubuntu-onlineaccounts,
78 qml-module-ubuntu-web (= ${binary:Version}),80 qml-module-ubuntu-web (= ${binary:Version}),
81 qtdeclarative5-ubuntu-content1,
82 qtdeclarative5-ubuntu-download-manager0.1,
79 qtdeclarative5-ubuntu-ui-toolkit-plugin (>= 1.3) | qtdeclarative5-ubuntu-ui-toolkit-plugin-gles (>= 1.3),83 qtdeclarative5-ubuntu-ui-toolkit-plugin (>= 1.3) | qtdeclarative5-ubuntu-ui-toolkit-plugin-gles (>= 1.3),
80 qtdeclarative5-unity-action-plugin,84 qtdeclarative5-unity-action-plugin,
81 unity-webapps-qml,85 unity-webapps-qml,
@@ -164,6 +168,7 @@
164Multi-Arch: foreign168Multi-Arch: foreign
165Depends: ${misc:Depends},169Depends: ${misc:Depends},
166 autopilot-qt5,170 autopilot-qt5,
171 content-hub-testability,
167 python3-autopilot,172 python3-autopilot,
168 python3-fixtures,173 python3-fixtures,
169 python3-psutil,174 python3-psutil,
170175
=== modified file 'tests/autopilot/webbrowser_app/tests/test_downloads.py'
--- tests/autopilot/webbrowser_app/tests/test_downloads.py 2016-11-03 13:09:29 +0000
+++ tests/autopilot/webbrowser_app/tests/test_downloads.py 2016-11-03 18:00:54 +0000
@@ -21,11 +21,12 @@
2121
22from testtools.matchers import Equals22from testtools.matchers import Equals
2323
24import ubuntuuitoolkit as uitk
25
26import subprocess
24import testtools27import testtools
2528
2629
27@testtools.skipIf(model() == "Desktop", "Don't run on desktop, as "
28 "dependencies aren't guaranteed")
29class TestDownloads(StartOpenRemotePageTestCaseBase):30class TestDownloads(StartOpenRemotePageTestCaseBase):
3031
31 def test_open_close_downloads_page(self):32 def test_open_close_downloads_page(self):
@@ -60,14 +61,6 @@
60 self.assertThat(options_dialog.visible, Eventually(Equals(True)))61 self.assertThat(options_dialog.visible, Eventually(Equals(True)))
61 self.main_window.click_cancel_download_button()62 self.main_window.click_cancel_download_button()
6263
63 def test_picker(self):
64 self.main_window.go_to_url(self.base_url + "/downloadpdf")
65 options_dialog = self.main_window.get_download_options_dialog()
66 self.assertThat(options_dialog.visible, Eventually(Equals(True)))
67 self.main_window.click_choose_app_button()
68 picker = self.main_window.get_peer_picker()
69 self.assertThat(picker.visible, Eventually(Equals(True)))
70
71 def test_download(self):64 def test_download(self):
72 self.main_window.go_to_url(self.base_url + "/downloadpdf")65 self.main_window.go_to_url(self.base_url + "/downloadpdf")
73 options_dialog = self.main_window.get_download_options_dialog()66 options_dialog = self.main_window.get_download_options_dialog()
@@ -111,3 +104,26 @@
111 # Check that there are no entries in the public downloads window104 # Check that there are no entries in the public downloads window
112 entries = public_downloads_page.get_download_entries()105 entries = public_downloads_page.get_download_entries()
113 self.assertThat(len(entries), Equals(0))106 self.assertThat(len(entries), Equals(0))
107
108
109class TestDownloadsWithContentHubTestability(StartOpenRemotePageTestCaseBase):
110 def setUp(self):
111 # Run content-hub-peer-hook-wrapper which ensures that
112 # content-hub-testability has been register for the ContentPeersModel
113
114 # Find arch path of content-hub-peer-hook-wrapper
115 path = ("/usr/lib/%s/content-hub/content-hub-peer-hook-wrapper" %
116 uitk.base.get_host_multiarch())
117
118 return_code = subprocess.check_call([path])
119 self.assertThat(return_code, Equals(0))
120
121 super(TestDownloadsWithContentHubTestability, self).setUp()
122
123 def test_picker(self):
124 self.main_window.go_to_url(self.base_url + "/downloadpdf")
125 options_dialog = self.main_window.get_download_options_dialog()
126 self.assertThat(options_dialog.visible, Eventually(Equals(True)))
127 self.main_window.click_choose_app_button()
128 picker = self.main_window.get_peer_picker()
129 self.assertThat(picker.visible, Eventually(Equals(True)))

Subscribers

People subscribed via source and target branches