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/ (+0/-1)
ubuntu_autopilot_tests/fileroller/ (+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:

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

Description of the change

Fixes 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

Hash: SHA256


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.
>You are the owner of

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


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


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, 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, 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' 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/'
2--- ubuntu_autopilot_tests/fileroller/ 2013-05-29 11:56:42 +0000
3+++ ubuntu_autopilot_tests/fileroller/ 2014-03-14 15:38:17 +0000
4@@ -1,1 +0,0 @@
5-# -*- coding: utf-8 -*-
7=== modified file 'ubuntu_autopilot_tests/fileroller/'
8--- ubuntu_autopilot_tests/fileroller/ 2013-11-22 22:00:27 +0000
9+++ ubuntu_autopilot_tests/fileroller/ 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
21+ from unittest import mock
22+except ImportError:
23+ import mock
24 import shutil
25 import tempfile
27+logger = logging.getLogger('__name__')
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()
39+ #launch the app
40 = self.launch_test_application('file-roller')
42 # only instantiate these once for multiple tests
43@@ -32,6 +44,12 @@
44 self.new_archive_button ='GtkToolButton',
45 name=u'New')
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))
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)
59 # TODO: Figure out a gzip commpress, does not have an active Id enabled
61 # select the create button and check it is clickable
62 self.create_button ='GtkButton',
63 label=u'create-archive')
64@@ -118,30 +135,24 @@
65 self.add_files_dialog ='FrFileSelectorDialog')
66 self.assertThat(self.add_files_dialog.title,
67 Eventually(Equals('Add Files')))
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
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")
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")
100 # We must be able to click the '_Add' button
101 self.add_folder_button ='GtkLabel',
102 label=u'_Add')
103@@ -186,6 +197,7 @@
105 self.keyboard.press_and_release("Alt+l")
106 self.keyboard.type(tempArchive)
107+ self.keyboard.press_and_release('Enter')
109 self.extract_archive_button =
110 'GtkLabel', label=u'_Extract')
111@@ -207,6 +219,12 @@
113 self.pointing_device.move_to_object(self.show_files_button)
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())
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)
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
148 #hardcoded global archiveName
149 def _tempArchiveName():


People subscribed via source and target branches