Merge lp:~adam-disc0tech/ubuntu-autopilot-tests/fileroller into lp:ubuntu-autopilot-tests

Proposed by Adam Smith
Status: Needs review
Proposed branch: lp:~adam-disc0tech/ubuntu-autopilot-tests/fileroller
Merge into: lp:ubuntu-autopilot-tests
Diff against target: 149 lines (+53/-17)
2 files modified
ubuntu_autopilot_tests/fileroller/__init__.py (+0/-1)
ubuntu_autopilot_tests/fileroller/test_fileroller.py (+53/-16)
To merge this branch: bzr merge lp:~adam-disc0tech/ubuntu-autopilot-tests/fileroller
Reviewer Review Type Date Requested Status
Nicholas Skaggs (community) Approve
Chris Gagnon Pending
Dan Chapman  Pending
Review via email: mp+211072@code.launchpad.net

This proposal supersedes a proposal from 2014-02-25.

Description of the change

Fixes https://bugs.launchpad.net/ubuntu-autopilot-tests/+bug/1206386 by doing two things

1) correcting the way content is selected for the archive
2) checking the correct content is extracted
3) home directory is now mocked

To post a comment you must log in.
Revision history for this message
Nicholas Skaggs (nskaggs) wrote : Posted in a previous version of this proposal

Thanks for the merge Adam. I have a few questions;

We can't use sudo in a test; this are non-interactive, and I'm confused why you would need root anyway. However, the symbolic linking to /home is very confusing to me; what are you trying to accomplish?

Adding logging and the 'check the files have actually been extracted' are both welcome additions.

review: Needs Fixing
Revision history for this message
Adam Smith (adam-disc0tech) wrote : Posted in a previous version of this proposal

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi

You are right, sudo shouldn't even be required.

What I'm trying to achieve, is to create a syncing in home with the name ___000, pointing to the content under /usr/share/example-content.

This seemed easier / safer to me than navigating the folder structure through the gui. This is the bit that seemed to be broken when I picked up the test. I can be pretty sure that the link is the first item listed in home.

- -adam

On 25 February 2014 21:30:51 GMT+00:00, Nicholas Skaggs <email address hidden> wrote:
>
>Review: Needs Fixing
>
>Thanks for the merge Adam. I have a few questions;
>
>We can't use sudo in a test; this are non-interactive, and I'm confused
>why you would need root anyway. However, the symbolic linking to /home
>is very confusing to me; what are you trying to accomplish?
>
>Adding logging and the 'check the files have actually been extracted'
>are both welcome additions.
>--
>https://code.launchpad.net/~adam-disc0tech/ubuntu-autopilot-tests/fileroller/+merge/208183
>You are the owner of
>lp:~adam-disc0tech/ubuntu-autopilot-tests/fileroller.

- --
Sent from my Android device with K-9 Mail. Please excuse my brevity.
-----BEGIN PGP SIGNATURE-----
Version: APG v1.0.9

iQI8BAEBCAAmBQJTDaQrHxxkaXNjMHRlY2ggPGFkYW1AZGlzYzB0ZWNoLmNvbT4A
CgkQveW9bcQ9QGwjtRAAiPgywHrGVnfnwYm2pxIEeYZHjbXQp2Y2UQ2WMebuB8iM
vvPDPxm6tPudxZ63QFooUNTS7Y3FiAfnyhLSDM93vCeIfysTBVZ6Va9WuT02B8Is
SyGaNLI4pV0PL4c/xNkK2FPWjJtL62W6Q9WxmYLjbi6jw+tKA6aa87QIsO6aioW/
2FgF3wtyMU24iYkNSStRls50EYiG9BflmYobWagAolpJ1ZfQEW6Os+nVMaaJULOY
ow9CLSitwm4huSNEhFk8pPb2QFH5T+YEhfIcXZA5OGOLJGbQ3uWE+2kAkDOlPSfk
hSUIREMcaqPIoW3/EidbOCU24cRUrl9Q901s6xYGBU1ql7wa7mbIFdNhqZk0hnJj
R/6QrN+LF14s0GeO/+GA4Jts+LTLhNHaxUau9W9Yq+r3nbLV9qs9//jW5+1TjUPF
31kF8PIol1AEehEqVtVCdOreEEd0dVvOaY/gX2K0LrkWY8WEBMPf9Rb8hDf5SoEe
XO/QzjUcSaobPNA3iNyS9aX4yprGfuVONjcTE3qkwZk17X4heA5EReVeA/8Nb2d1
83Ga7EqDCkN7IlhCdDufwM+od7d6lIHNmkucDSghwjPRgSdo/jZaaUJGT0UvOI0W
oZmuH9ZHp39EjrGhoaQncru/3X24WfZBs+gfwpsbwPXBWkdcVBZIFnTr1YTQCdo=
=lWOT
-----END PGP SIGNATURE-----

Revision history for this message
Adam Smith (adam-disc0tech) wrote : Posted in a previous version of this proposal

Fixed

Revision history for this message
Nicholas Skaggs (nskaggs) wrote : Posted in a previous version of this proposal

I'm still not sure on using ___000 in home. Let me suggest something we can both approve of; mocking /home and doing the same thing (though you can pick a saner name as your folder will be the only one in the new "/home"). This will ensure we don't leave any remenants of testing behind, nor mangle with /home dirs. Be sure and put the mock location as a temporary dir in /tmp.

You can do this with python mock. For an example see http://bazaar.launchpad.net/~music-app-dev/music-app/trunk/view/head:/tests/autopilot/music_app/tests/__init__.py, specifically in regards to _patch_home.

review: Needs Fixing
Revision history for this message
Chris Gagnon (chris.gagnon) wrote : Posted in a previous version of this proposal

I saw this in my email and thought I should comment.

import's should not be on the same line according to pep8 http://legacy.python.org/dev/peps/pep-0008/#imports, it's also good practice to keep them in alphabetical order.

13 +import os, subprocess
14 +import shutil, logging
15 import tempfile

should be:

import logging
import os
import shutil
import subprocess
import tempfile

if you apt-get install python-flake8 you can run 'flake8 some_file.py' and it will tell you about the pep8 style guides problems in your code.

A lot of code editors have a flake8 plugin that will tell you when you have errors, so you don't have to do it manually.

review: Needs Fixing
Revision history for this message
Adam Smith (adam-disc0tech) wrote :

All comments above taken into account, PEP8 compliance done, home directory mocked, ___000 references removed, no more symlink...

72. By Adam Smith

removed redundant method

73. By Adam Smith

Removed 25 character temp directory code, not required for this app test

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

BTW, copying the XAuthority file is for jenkins, so i suppose we can retain it for now. I approve the clean-up, it's a step in the right direction. Any chance for removing the sleeps and checking for status instead?

review: Approve

Unmerged revisions

73. By Adam Smith

Removed 25 character temp directory code, not required for this app test

72. By Adam Smith

removed redundant method

71. By Adam Smith

Implemented fake home directory

70. By Adam Smith

Syntax issue fixed

69. By Adam Smith

PEP8 compliance issues (mostly) fixed

68. By Adam Smith

Removed unnecessary use of root

67. By Adam Smith

Hit UP a lot when you enter the file dialog to ensure the same starting location

66. By Adam Smith

Added to step to go back to home directory each time the file dialog is opened

65. By Adam Smith

Changed mechanism for identifying content to archive.

64. By Adam Smith

Added assertions to check files and directories are actually extracted

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'ubuntu_autopilot_tests/fileroller/__init__.py'
--- ubuntu_autopilot_tests/fileroller/__init__.py 2013-05-29 11:56:42 +0000
+++ ubuntu_autopilot_tests/fileroller/__init__.py 2014-03-14 15:38:17 +0000
@@ -1,1 +0,0 @@
1# -*- coding: utf-8 -*-
20
=== modified file 'ubuntu_autopilot_tests/fileroller/test_fileroller.py'
--- ubuntu_autopilot_tests/fileroller/test_fileroller.py 2013-11-22 22:00:27 +0000
+++ ubuntu_autopilot_tests/fileroller/test_fileroller.py 2014-03-14 15:38:17 +0000
@@ -1,12 +1,20 @@
1from autopilot.testcase import AutopilotTestCase1from autopilot.testcase import AutopilotTestCase
2from autopilot.matchers import Eventually2from autopilot.matchers import Eventually
3from testtools.matchers import Equals, Contains, NotEquals3from testtools.matchers import Equals, Contains
4from testtools.matchers import NotEquals, DirExists, FileExists
4from autopilot.input import Mouse, Pointer5from autopilot.input import Mouse, Pointer
5from time import sleep6from time import sleep
7import logging
6import os8import os
9try:
10 from unittest import mock
11except ImportError:
12 import mock
7import shutil13import shutil
8import tempfile14import tempfile
915
16logger = logging.getLogger('__name__')
17
1018
11class FileRollerTests(AutopilotTestCase):19class FileRollerTests(AutopilotTestCase):
12 '''Collection of autopilot test for file-roller (Archive Manager) '''20 '''Collection of autopilot test for file-roller (Archive Manager) '''
@@ -14,6 +22,10 @@
14 def setUp(self):22 def setUp(self):
15 '''Set-up method'''23 '''Set-up method'''
16 super(FileRollerTests, self).setUp()24 super(FileRollerTests, self).setUp()
25 #mock out the home dir
26 self.home_dir = self._patch_home()
27
28 #launch the app
17 self.app = self.launch_test_application('file-roller')29 self.app = self.launch_test_application('file-roller')
1830
19 # only instantiate these once for multiple tests31 # only instantiate these once for multiple tests
@@ -32,6 +44,12 @@
32 self.new_archive_button = self.app.select_single('GtkToolButton',44 self.new_archive_button = self.app.select_single('GtkToolButton',
33 name=u'New')45 name=u'New')
3446
47 #create symbolic link for fast test content location
48 src_dir = '/usr/share/example-content/Ubuntu_Free_Culture_Showcase/'
49 files = os.listdir(src_dir)
50 for f in files:
51 shutil.copyfile(src_dir + str(f), os.getenv("HOME") + '/' + str(f))
52
35 def test_create_archive(self):53 def test_create_archive(self):
36 '''Creates an archive and adds sample data,54 '''Creates an archive and adds sample data,
37 Window title must contain archive name'''55 Window title must contain archive name'''
@@ -103,7 +121,6 @@
103 self.keyboard.type(tempArchive)121 self.keyboard.type(tempArchive)
104122
105 # TODO: Figure out a gzip commpress, does not have an active Id enabled123 # TODO: Figure out a gzip commpress, does not have an active Id enabled
106
107 # select the create button and check it is clickable124 # select the create button and check it is clickable
108 self.create_button = self.app.select_single('GtkButton',125 self.create_button = self.app.select_single('GtkButton',
109 label=u'create-archive')126 label=u'create-archive')
@@ -118,30 +135,24 @@
118 self.add_files_dialog = self.app.select_single('FrFileSelectorDialog')135 self.add_files_dialog = self.app.select_single('FrFileSelectorDialog')
119 self.assertThat(self.add_files_dialog.title,136 self.assertThat(self.add_files_dialog.title,
120 Eventually(Equals('Add Files')))137 Eventually(Equals('Add Files')))
121
122 # This is a horrible hacky workaround to add sample138 # This is a horrible hacky workaround to add sample
123 # files from /usr/share/example-content/*139 # files from /usr/share/example-content/*
124 # Need to find a better workaround :S Location GtkEntry140 # Need to find a better workaround :S Location GtkEntry
125 # doesnt work when entering file path141 # doesnt work when entering file path
126 # Maybe add some assertions to check that the142 # Maybe add some assertions to check that the
127 # Location GTKEntry contains correct path143 # Location GTKEntry contains correct path
144 #first ensure we are in the home directory
145
128 self.keyboard.press_and_release("Alt+l")146 self.keyboard.press_and_release("Alt+l")
129 self.keyboard.press_and_release("Tab")147 self.keyboard.press_and_release("Tab")
130 self.keyboard.press_and_release("Tab")148 self.keyboard.press_and_release("Tab")
131 self.keyboard.type("File")
132 self.keyboard.press_and_release("Enter")
133 self.keyboard.press_and_release("Tab")149 self.keyboard.press_and_release("Tab")
134 self.keyboard.press_and_release("Right")150
135 self.keyboard.press_and_release("Right")151 self.keyboard.press_and_release("Space")
136 self.keyboard.type("usr")152 self.keyboard.press_and_release("Down")
137 self.keyboard.press_and_release("Enter")153 self.keyboard.press_and_release("Down")
138 self.keyboard.type("share")154 self.keyboard.press_and_release("Space")
139 self.keyboard.press_and_release("Enter")155
140 self.keyboard.type("example-content")
141 self.keyboard.press_and_release("Enter")
142 self.keyboard.press_and_release("Left")
143 self.keyboard.press_and_release("Left")
144 self.keyboard.press_and_release("Enter")
145 # We must be able to click the '_Add' button156 # We must be able to click the '_Add' button
146 self.add_folder_button = self.app.select_single('GtkLabel',157 self.add_folder_button = self.app.select_single('GtkLabel',
147 label=u'_Add')158 label=u'_Add')
@@ -186,6 +197,7 @@
186197
187 self.keyboard.press_and_release("Alt+l")198 self.keyboard.press_and_release("Alt+l")
188 self.keyboard.type(tempArchive)199 self.keyboard.type(tempArchive)
200 self.keyboard.press_and_release('Enter')
189201
190 self.extract_archive_button = self.app.select_single(202 self.extract_archive_button = self.app.select_single(
191 'GtkLabel', label=u'_Extract')203 'GtkLabel', label=u'_Extract')
@@ -207,6 +219,12 @@
207219
208 self.pointing_device.move_to_object(self.show_files_button)220 self.pointing_device.move_to_object(self.show_files_button)
209 self.pointing_device.click()221 self.pointing_device.click()
222
223 #check the files have actually been extracted
224 self.assertThat(tempArchive, DirExists())
225 self.assertThat(tempArchive + '/' + '/How fast.ogg', FileExists())
226 self.assertThat(tempArchive + '/' + '/Josh Woodward - Swansong.ogg', FileExists())
227
210 # This sleep is needed as we can't introspect228 # This sleep is needed as we can't introspect
211 # nautilus when it shows the files229 # nautilus when it shows the files
212 # TODO: Once nautilus can be introspected we need to lose this sleep230 # TODO: Once nautilus can be introspected we need to lose this sleep
@@ -254,6 +272,25 @@
254 extract_path = os.path.join('/tmp/' + fileName)272 extract_path = os.path.join('/tmp/' + fileName)
255 self.addCleanup(shutil.rmtree, extract_path)273 self.addCleanup(shutil.rmtree, extract_path)
256274
275 def _patch_home(self):
276 #make a temp dir
277 temp_dir = tempfile.mkdtemp()
278 logger.debug("Created fake home directory " + temp_dir)
279 self.addCleanup(shutil.rmtree, temp_dir)
280 #if the Xauthority file is in home directory
281 #make sure we copy it to temp home, otherwise do nothing
282 xauth = os.path.expanduser(os.path.join('~', '.Xauthority'))
283 if os.path.isfile(xauth):
284 logger.debug("Copying .Xauthority to fake home " + temp_dir)
285 shutil.copyfile(
286 os.path.expanduser(os.path.join('~', '.Xauthority')),
287 os.path.join(temp_dir, '.Xauthority'))
288 patcher = mock.patch.dict('os.environ', {'HOME': temp_dir})
289 patcher.start()
290 logger.debug("Patched home to fake home directory " + temp_dir)
291 self.addCleanup(patcher.stop)
292 return temp_dir
293
257294
258#hardcoded global archiveName295#hardcoded global archiveName
259def _tempArchiveName():296def _tempArchiveName():

Subscribers

People subscribed via source and target branches