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

Proposed by Adam Smith on 2014-03-14
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) 2014-03-14 Approve on 2014-03-14
Dan Chapman  2014-03-14 Pending
Chris Gagnon 2014-03-14 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.
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
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-----

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

Fixed

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
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
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 on 2014-03-14

removed redundant method

73. By Adam Smith on 2014-03-14

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

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 on 2014-03-14

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

72. By Adam Smith on 2014-03-14

removed redundant method

71. By Adam Smith on 2014-03-14

Implemented fake home directory

70. By Adam Smith on 2014-03-14

Syntax issue fixed

69. By Adam Smith on 2014-03-14

PEP8 compliance issues (mostly) fixed

68. By Adam Smith on 2014-03-02

Removed unnecessary use of root

67. By Adam Smith on 2014-02-25

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

66. By Adam Smith on 2014-02-25

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

65. By Adam Smith on 2014-02-25

Changed mechanism for identifying content to archive.

64. By Adam Smith on 2014-02-25

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
1=== modified file 'ubuntu_autopilot_tests/fileroller/__init__.py'
2--- ubuntu_autopilot_tests/fileroller/__init__.py 2013-05-29 11:56:42 +0000
3+++ ubuntu_autopilot_tests/fileroller/__init__.py 2014-03-14 15:38:17 +0000
4@@ -1,1 +0,0 @@
5-# -*- coding: utf-8 -*-
6
7=== modified file 'ubuntu_autopilot_tests/fileroller/test_fileroller.py'
8--- ubuntu_autopilot_tests/fileroller/test_fileroller.py 2013-11-22 22:00:27 +0000
9+++ ubuntu_autopilot_tests/fileroller/test_fileroller.py 2014-03-14 15:38:17 +0000
10@@ -1,12 +1,20 @@
11 from autopilot.testcase import AutopilotTestCase
12 from autopilot.matchers import Eventually
13-from testtools.matchers import Equals, Contains, NotEquals
14+from testtools.matchers import Equals, Contains
15+from testtools.matchers import NotEquals, DirExists, FileExists
16 from autopilot.input import Mouse, Pointer
17 from time import sleep
18+import logging
19 import os
20+try:
21+ from unittest import mock
22+except ImportError:
23+ import mock
24 import shutil
25 import tempfile
26
27+logger = logging.getLogger('__name__')
28+
29
30 class FileRollerTests(AutopilotTestCase):
31 '''Collection of autopilot test for file-roller (Archive Manager) '''
32@@ -14,6 +22,10 @@
33 def setUp(self):
34 '''Set-up method'''
35 super(FileRollerTests, self).setUp()
36+ #mock out the home dir
37+ self.home_dir = self._patch_home()
38+
39+ #launch the app
40 self.app = self.launch_test_application('file-roller')
41
42 # only instantiate these once for multiple tests
43@@ -32,6 +44,12 @@
44 self.new_archive_button = self.app.select_single('GtkToolButton',
45 name=u'New')
46
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+
53 def test_create_archive(self):
54 '''Creates an archive and adds sample data,
55 Window title must contain archive name'''
56@@ -103,7 +121,6 @@
57 self.keyboard.type(tempArchive)
58
59 # TODO: Figure out a gzip commpress, does not have an active Id enabled
60-
61 # select the create button and check it is clickable
62 self.create_button = self.app.select_single('GtkButton',
63 label=u'create-archive')
64@@ -118,30 +135,24 @@
65 self.add_files_dialog = self.app.select_single('FrFileSelectorDialog')
66 self.assertThat(self.add_files_dialog.title,
67 Eventually(Equals('Add Files')))
68-
69 # This is a horrible hacky workaround to add sample
70 # files from /usr/share/example-content/*
71 # Need to find a better workaround :S Location GtkEntry
72 # doesnt work when entering file path
73 # Maybe add some assertions to check that the
74 # Location GTKEntry contains correct path
75+ #first ensure we are in the home directory
76+
77 self.keyboard.press_and_release("Alt+l")
78 self.keyboard.press_and_release("Tab")
79 self.keyboard.press_and_release("Tab")
80- self.keyboard.type("File")
81- self.keyboard.press_and_release("Enter")
82 self.keyboard.press_and_release("Tab")
83- self.keyboard.press_and_release("Right")
84- self.keyboard.press_and_release("Right")
85- self.keyboard.type("usr")
86- self.keyboard.press_and_release("Enter")
87- self.keyboard.type("share")
88- self.keyboard.press_and_release("Enter")
89- self.keyboard.type("example-content")
90- self.keyboard.press_and_release("Enter")
91- self.keyboard.press_and_release("Left")
92- self.keyboard.press_and_release("Left")
93- self.keyboard.press_and_release("Enter")
94+
95+ self.keyboard.press_and_release("Space")
96+ self.keyboard.press_and_release("Down")
97+ self.keyboard.press_and_release("Down")
98+ self.keyboard.press_and_release("Space")
99+
100 # We must be able to click the '_Add' button
101 self.add_folder_button = self.app.select_single('GtkLabel',
102 label=u'_Add')
103@@ -186,6 +197,7 @@
104
105 self.keyboard.press_and_release("Alt+l")
106 self.keyboard.type(tempArchive)
107+ self.keyboard.press_and_release('Enter')
108
109 self.extract_archive_button = self.app.select_single(
110 'GtkLabel', label=u'_Extract')
111@@ -207,6 +219,12 @@
112
113 self.pointing_device.move_to_object(self.show_files_button)
114 self.pointing_device.click()
115+
116+ #check the files have actually been extracted
117+ self.assertThat(tempArchive, DirExists())
118+ self.assertThat(tempArchive + '/' + '/How fast.ogg', FileExists())
119+ self.assertThat(tempArchive + '/' + '/Josh Woodward - Swansong.ogg', FileExists())
120+
121 # This sleep is needed as we can't introspect
122 # nautilus when it shows the files
123 # TODO: Once nautilus can be introspected we need to lose this sleep
124@@ -254,6 +272,25 @@
125 extract_path = os.path.join('/tmp/' + fileName)
126 self.addCleanup(shutil.rmtree, extract_path)
127
128+ def _patch_home(self):
129+ #make a temp dir
130+ temp_dir = tempfile.mkdtemp()
131+ logger.debug("Created fake home directory " + temp_dir)
132+ self.addCleanup(shutil.rmtree, temp_dir)
133+ #if the Xauthority file is in home directory
134+ #make sure we copy it to temp home, otherwise do nothing
135+ xauth = os.path.expanduser(os.path.join('~', '.Xauthority'))
136+ if os.path.isfile(xauth):
137+ logger.debug("Copying .Xauthority to fake home " + temp_dir)
138+ shutil.copyfile(
139+ os.path.expanduser(os.path.join('~', '.Xauthority')),
140+ os.path.join(temp_dir, '.Xauthority'))
141+ patcher = mock.patch.dict('os.environ', {'HOME': temp_dir})
142+ patcher.start()
143+ logger.debug("Patched home to fake home directory " + temp_dir)
144+ self.addCleanup(patcher.stop)
145+ return temp_dir
146+
147
148 #hardcoded global archiveName
149 def _tempArchiveName():

Subscribers

People subscribed via source and target branches