Merge lp:~diegosarmentero/ubuntuone-client/fix-user-home-tests into lp:ubuntuone-client

[Diego Sarmentero]

- Fixed: tests are now not modifying the real user home (LP: #915380).

Just some fixes in the tests.

[lp:~diegosarmentero/ubuntuone-client/fix-user-home-tests]

1181. By Diego Sarmentero on 2012-01-17

Tests Fixed. Home patching made in BaseTwistedTestCase

1182. By Diego Sarmentero on 2012-01-18

fixed tests.

[Natalia Bidart (nataliabidart)]

* Though FakeMainTestCase used to use self.tmpdir as homke dir, it is more correct to use a dedicated dir for that, so we can isolate home data into a dir (we may need that to debug a test).

Can you please make self.home_dir = self.mktemp('ubuntuonehacker')?

* Also, in tests/syncdaemon/test_action_queue.py you're removing the patch os.environ['HOME'] = self.home, which is great, but the self.home variable should point to the same home that the base test case is using to patch the xdg_home var. So, I would guess that in BasicTestCase we need to change:

        self.home = self.mktemp('home')

for

        self.home = self.home_dir

(please confirm this makes sense before changing it, I did not reviewed the whole test module).

* There are several tests that are re-defining self.home_dir and re-patching xdg_home... can you please remove that? (we need to confirm they all inherit from BaseTwistedTestCase either directly or by some inheritance chain). A quick grep gives me:

nessita@dali:~/canonical/client/review_fix-user-home-tests$ grep -rn --color "patch.*xdg_home" *
contrib/testing/testcase.py:383: self.patch(platform, "xdg_home", self.home_dir)
tests/syncdaemon/test_localrescan.py:134: self.patch(platform, 'xdg_home', self.home_dir)
tests/syncdaemon/test_localrescan.py:374: self.patch(platform, 'xdg_home', self.home_dir)
tests/syncdaemon/test_localrescan.py:2021: self.patch(platform, 'xdg_home', self.home_dir)
tests/syncdaemon/test_localrescan.py:2260: self.patch(platform, 'xdg_home', self.home_dir)
tests/syncdaemon/test_config.py:534: self.patch(platform, 'xdg_home', homedir)
tests/syncdaemon/test_main.py:267: self.patch(main_mod, "xdg_home", self.home_dir)
tests/syncdaemon/test_eq_inotify.py:664: self.patch(platform, 'xdg_home', self.home_dir)
tests/platform/test_os_helper.py:95: self.patch(platform, "xdg_home", self.my_home)
nessita@dali:~/canonical/client/review_fix-user-home-tests$

nessita@dali:~/canonical/client/review_fix-user-home-tests$ grep -rn --color "self\.home_dir =" *
contrib/testing/testcase.py:382: self.home_dir = self.tmpdir
tests/syncdaemon/test_vm.py:3739: self.home_dir = self.mktemp('ubuntuonehacker')
tests/syncdaemon/test_localrescan.py:126: self.home_dir = self.mktemp('ubuntuonehacker')
tests/syncdaemon/test_eventqueue.py:46: self.home_dir = self.mktemp('home_dir')
tests/syncdaemon/test_eq_inotify.py:649: self.home_dir = self.mktemp('ubuntuonehacker')
tests/platform/linux/test_vm.py:464: self.home_dir = os.path.join(self.tmpdir, 'home', 'ubuntuonehacker')
tests/platform/linux/test_filesystem_notifications.py:102: self.home_dir = self.mktemp('home_dir')
tests/platform/test_filesystem_notifications.py:102: self.home_dir = self.mktemp('home_dir')
nessita@dali:~/canonical/client/review_fix-user-home-tests$

review: Needs Fixing

[lp:~diegosarmentero/ubuntuone-client/fix-user-home-tests]

1183. By Diego Sarmentero on 2012-01-18

Refactor involving home_dir

1184. By Diego Sarmentero on 2012-01-18

Remove commented lines

1185. By Diego Sarmentero on 2012-01-19

merge

[Natalia Bidart (nataliabidart)]

I'm getting all these failures in Linux http://pastebin.ubuntu.com/809586/

review: Needs Fixing

[lp:~diegosarmentero/ubuntuone-client/fix-user-home-tests]

1186. By Diego Sarmentero on 2012-01-23

merge

1187. By Diego Sarmentero on 2012-01-23

Tests fixed

[Brian Curtin (brian.curtin)]

This looks better to me.

[Brian Curtin (brian.curtin)]

(forgot to approve)

review: Approve

[Natalia Bidart (nataliabidart)]

Looks good!

review: Approve

=== modified file 'contrib/testing/testcase.py'
--- contrib/testing/testcase.py	2011-12-29 14:44:10 +0000
+++ contrib/testing/testcase.py	2012-01-23 20:01:25 +0000
@@ -48,6 +48,7 @@
     tritcask,
 )
 from ubuntuone.syncdaemon import logger
+from ubuntuone import platform
 from ubuntuone.platform import (
     can_write,
     make_dir,
@@ -377,6 +378,9 @@
         self.log = logging.getLogger("ubuntuone.SyncDaemon.TEST")
         self.log.info("starting test %s.%s", self.__class__.__name__,
                       self._testMethodName)
+        # Patch the user home
+        self.home_dir = self.mktemp('ubuntuonehacker')
+        self.patch(platform, "xdg_home", self.home_dir)
 
 
 class FakeMainTestCase(BaseTwistedTestCase):
@@ -389,11 +393,6 @@
         """Setup the infrastructure for the test."""
         yield super(FakeMainTestCase, self).setUp()
 
-        self.home_dir = self.tmpdir
-        old_home = os.environ['HOME']
-        os.environ['HOME'] = self.home_dir
-        self.addCleanup(os.environ.__setitem__, 'HOME', old_home)
-
         self.data_dir = self.mktemp('data_dir')
         self.partials_dir = self.mktemp('partials')
         self.root_dir = self.mktemp('root_dir')

=== modified file 'tests/platform/linux/eventlog/test_zg_listener.py'
--- tests/platform/linux/eventlog/test_zg_listener.py	2011-11-09 20:03:15 +0000
+++ tests/platform/linux/eventlog/test_zg_listener.py	2012-01-23 20:01:25 +0000
@@ -368,20 +368,6 @@
 class ZeitgeistUDFsTestCase(ZeitgeistListenerTestCase):
     """Tests for all UDFs-related zeitgeist events."""
 
-    @defer.inlineCallbacks
-    def setUp(self):
-        """Initialize this test instance."""
-        yield super(ZeitgeistUDFsTestCase, self).setUp()
-        self.home_dir = self.mktemp('ubuntuonehacker')
-        self._old_home = os.environ['HOME']
-        os.environ['HOME'] = self.home_dir
-
-    @defer.inlineCallbacks
-    def tearDown(self):
-        """Finalize this test instance."""
-        os.environ['HOME'] = self._old_home
-        yield super(ZeitgeistUDFsTestCase, self).tearDown()
-
     def _create_udf(self, id, node_id, suggested_path, subscribed=True):
         """Create an UDF and returns it and the volume."""
         path = get_udf_path(suggested_path)

=== modified file 'tests/platform/linux/test_filesystem_notifications.py'
--- tests/platform/linux/test_filesystem_notifications.py	2011-09-02 16:09:41 +0000
+++ tests/platform/linux/test_filesystem_notifications.py	2012-01-23 20:01:25 +0000
@@ -99,7 +99,6 @@
         fsmdir = self.mktemp('fsmdir')
         partials_dir = self.mktemp('partials_dir')
         self.root_dir = self.mktemp('root_dir')
-        self.home_dir = self.mktemp('home_dir')
         self.vm = testcase.FakeVolumeManager(self.root_dir)
         self.tritcask_dir = self.mktemp("tritcask_dir")
         self.db = Tritcask(self.tritcask_dir)

=== modified file 'tests/platform/linux/test_vm.py'
--- tests/platform/linux/test_vm.py	2011-11-17 19:19:08 +0000
+++ tests/platform/linux/test_vm.py	2012-01-23 20:01:25 +0000
@@ -459,9 +459,11 @@
     @defer.inlineCallbacks
     def setUp(self):
         yield super(MetadataNewLayoutTests, self).setUp()
-        self.share_md_dir = os.path.join(self.vm_data_dir, 'shares')
-        self.shared_md_dir = os.path.join(self.vm_data_dir, 'shared')
+        # We need to define home_dir here to add 'home' to the path
+        # and avoid crashes between existing paths.
         self.home_dir = os.path.join(self.tmpdir, 'home', 'ubuntuonehacker')
+        self.share_md_dir = os.path.join(self.vm_data_dir, 'shares')
+        self.shared_md_dir = os.path.join(self.vm_data_dir, 'shared')
         self.u1_dir = os.path.join(self.home_dir, os.path.split(self.u1_dir)[1])
         self.root_dir = self.u1_dir
         self.shares_dir = os.path.join(self.tmpdir, 'shares')

=== modified file 'tests/platform/test_filesystem_notifications.py'
--- tests/platform/test_filesystem_notifications.py	2011-10-21 13:40:09 +0000
+++ tests/platform/test_filesystem_notifications.py	2012-01-23 20:01:25 +0000
@@ -99,7 +99,6 @@
         fsmdir = self.mktemp('fsmdir')
         partials_dir = self.mktemp('partials_dir')
         self.root_dir = self.mktemp('root_dir')
-        self.home_dir = self.mktemp('home_dir')
         self.vm = FakeVolumeManager(self.root_dir)
         self.tritcask_dir = self.mktemp("tritcask_dir")
         self.db = Tritcask(self.tritcask_dir)

=== modified file 'tests/platform/test_os_helper.py'
--- tests/platform/test_os_helper.py	2012-01-12 14:01:38 +0000
+++ tests/platform/test_os_helper.py	2012-01-23 20:01:25 +0000
@@ -28,7 +28,6 @@
     BaseTwistedTestCase,
     skip_if_win32_and_uses_readonly,
 )
-from ubuntuone import platform
 from ubuntuone.platform import (
     access,
     allow_writes,
@@ -91,8 +90,6 @@
         yield super(OSWrapperTests, self).setUp(
             test_dir_name=test_dir_name, test_file_name=test_file_name,
             valid_file_path_builder=valid_file_path_builder)
-        self.my_home = 'myhome'
-        self.patch(platform, "xdg_home", self.my_home)
         # make sure the file exists
         open_file(self.testfile, 'w').close()
 
@@ -395,21 +392,21 @@
         """Test the expand_user function with a path like: ~/userpath."""
         path = os.path.join('~', 'userpath')
         result = expand_user(path)
-        expected = os.path.join(self.my_home, 'userpath')
+        expected = os.path.join(self.home_dir, 'userpath')
         self.assertEqual(expected, result)
 
     def test_expand_user_tilde_and_backslash(self):
         """Test the expand_user function with tilde and backslash."""
         tilde = '~' + os.path.sep
         result = expand_user(tilde)
-        expected = self.my_home + os.path.sep
+        expected = self.home_dir + os.path.sep
         self.assertEqual(expected, result)
 
     def test_expand_user_only_tilde(self):
         """Test the expand_user function returns with only tilde input."""
         tilde = '~'
         result = expand_user(tilde)
-        self.assertEqual(self.my_home, result)
+        self.assertEqual(self.home_dir, result)
         self.assertFalse(result.endswith(os.path.sep))
 
     def test_expand_user_fails_if_not_bytes(self):

=== modified file 'tests/platform/test_tools.py'
--- tests/platform/test_tools.py	2012-01-11 21:40:51 +0000
+++ tests/platform/test_tools.py	2012-01-23 20:01:25 +0000
@@ -31,7 +31,6 @@
     states,
     volume_manager,
 )
-from ubuntuone import platform
 from ubuntuone.platform import tools
 from tests.platform import IPCTestCase
 
@@ -533,7 +532,6 @@
     @defer.inlineCallbacks
     def test_create_folder(self):
         """Test for Folders.create."""
-        self.patch(platform, 'xdg_home', self.home_dir)
         path = os.path.join(self.home_dir, u'ñoño')
         volume_id = 'volume_id'
         node_id = 'node_id'
@@ -552,7 +550,6 @@
     @defer.inlineCallbacks
     def test_create_folder_error(self):
         """Test for Folders.create with error."""
-        self.patch(platform, 'xdg_home', self.home_dir)
         path = os.path.join(self.home_dir, u'ñoño')
 
         def create_udf(path, name, marker):

=== modified file 'tests/syncdaemon/test_action_queue.py'
--- tests/syncdaemon/test_action_queue.py	2011-12-20 10:58:38 +0000
+++ tests/syncdaemon/test_action_queue.py	2012-01-23 20:01:25 +0000
@@ -219,8 +219,8 @@
         """Init."""
         yield super(BasicTestCase, self).setUp()
 
+        self.home = self.home_dir
         self.root = self.mktemp('root')
-        self.home = self.mktemp('home')
         self.data = self.mktemp('data')
         self.shares = self.mktemp('shares')
         self.partials = self.mktemp('partials')
@@ -2532,18 +2532,6 @@
         """Init."""
         yield super(FilterEventsTestCase, self).setUp()
         self.vm = self.main.vm
-        self.old_home = os.environ.get('HOME', None)
-        os.environ['HOME'] = self.home
-
-    @defer.inlineCallbacks
-    def tearDown(self):
-        """Clean up."""
-        if self.old_home is None:
-            os.environ.pop('HOME')
-        else:
-            os.environ['HOME'] = self.old_home
-
-        yield super(FilterEventsTestCase, self).tearDown()
 
 
 class ChangePublicAccessTests(ConnectedBaseTestCase):

=== modified file 'tests/syncdaemon/test_eq_inotify.py'
--- tests/syncdaemon/test_eq_inotify.py	2011-11-30 19:54:45 +0000
+++ tests/syncdaemon/test_eq_inotify.py	2012-01-23 20:01:25 +0000
@@ -34,7 +34,6 @@
     skip_if_win32_missing_fs_event,
 )
 from tests.syncdaemon.test_eventqueue import BaseEQTestCase
-from ubuntuone import platform
 from ubuntuone.platform import (
     make_link,
     make_dir,
@@ -646,7 +645,6 @@
         """Init."""
         yield super(AncestorsUDFTestCase, self).setUp()
         self._deferred = defer.Deferred()
-        self.home_dir = self.mktemp('ubuntuonehacker')
         self.root_dir = self.mktemp('root_dir')
         self.data_dir = self.mktemp('data_dir')
         self.shares_dir = self.mktemp('shares_dir')
@@ -661,8 +659,6 @@
         self.eq.subscribe(self.listener)
         self.addCleanup(self.eq.unsubscribe, self.listener)
 
-        self.patch(platform, 'xdg_home', self.home_dir)
-
         # create UDF
         suggested_path = u'~/Documents/Reading/Books/PDFs'
         udf_id, node_id = 'udf_id', 'node_id'

=== modified file 'tests/syncdaemon/test_eventqueue.py'
--- tests/syncdaemon/test_eventqueue.py	2011-08-17 13:11:06 +0000
+++ tests/syncdaemon/test_eventqueue.py	2012-01-23 20:01:25 +0000
@@ -43,7 +43,6 @@
         self.fsmdir = self.mktemp('fsmdir')
         self.partials_dir = self.mktemp('partials_dir')
         self.root_dir = self.mktemp('root_dir')
-        self.home_dir = self.mktemp('home_dir')
         self.vm = FakeVolumeManager(self.root_dir)
         self.db = tritcask.Tritcask(self.mktemp('tritcask'))
         self.addCleanup(self.db.shutdown)

=== modified file 'tests/syncdaemon/test_localrescan.py'
--- tests/syncdaemon/test_localrescan.py	2011-11-30 19:54:45 +0000
+++ tests/syncdaemon/test_localrescan.py	2012-01-23 20:01:25 +0000
@@ -33,7 +33,6 @@
     FakeVolumeManager,
     skip_if_win32_and_uses_readonly,
 )
-from ubuntuone import platform
 from ubuntuone.platform import (
     make_dir,
     make_link,
@@ -123,16 +122,12 @@
     def setUp(self):
         """ Setup the test """
         yield super(BaseTestCase, self).setUp()
-        self.home_dir = self.mktemp('ubuntuonehacker')
         self.shares_dir = self.mktemp('shares')
         usrdir = self.mktemp("usrdir")
         self.fsmdir = self.mktemp("fsmdir")
         self.partials_dir = self.mktemp("partials")
         self.tritcask_dir = self.mktemp("tritcask")
 
-        # set the home for the tests
-        self.patch(platform, 'xdg_home', self.home_dir)
-
         self.vm = FakeVolumeManager(usrdir)
         self.db = Tritcask(self.tritcask_dir)
         self.addCleanup(self.db.shutdown)
@@ -371,8 +366,6 @@
         """Init."""
         yield super(VolumeTestCase, self).setUp()
 
-        self.patch(platform, 'xdg_home', self.home_dir)
-
         self.lr = local_rescan.LocalRescan(self.vm, self.fsm, self.eq, self.aq)
 
         self.volumes = []
@@ -2016,11 +2009,6 @@
     """Test what happens with volume roots left in a bad state last time."""
 
     @defer.inlineCallbacks
-    def setUp(self):
-        yield super(RootBadStateTests, self).setUp()
-        self.patch(platform, 'xdg_home', self.home_dir)
-
-    @defer.inlineCallbacks
     def _test_it(self, volume):
         """Run the bad state test for a specific volume."""
         path = volume.path
@@ -2257,8 +2245,6 @@
 
         self.lr = local_rescan.LocalRescan(self.vm, self.fsm, self.eq, self.aq)
 
-        self.patch(platform, 'xdg_home', self.home_dir)
-
         # create UDF
         suggested_path = u'~/Documents/Reading/Books/PDFs'
         udf_id, node_id = 'udf_id', 'node_id'

=== modified file 'tests/syncdaemon/test_main.py'
--- tests/syncdaemon/test_main.py	2012-01-03 12:40:04 +0000
+++ tests/syncdaemon/test_main.py	2012-01-23 20:01:25 +0000
@@ -264,6 +264,7 @@
 
     def test_get_homedir(self):
         """The get_homedir returns the root dir."""
+        self.patch(main_mod, "xdg_home", self.home_dir)
         expected = expand_user('~')
         main = self.build_main()
         self.assertEqual(main.get_homedir(), expected)

=== modified file 'tests/syncdaemon/test_vm.py'
--- tests/syncdaemon/test_vm.py	2011-12-12 19:32:25 +0000
+++ tests/syncdaemon/test_vm.py	2012-01-23 20:01:25 +0000
@@ -91,14 +91,12 @@
         self.log = logging.getLogger("ubuntuone.SyncDaemon.TEST")
         self.log.info("starting test %s.%s", self.__class__.__name__,
                       self._testMethodName)
-        self.home_dir = self.mktemp('ubuntuonehacker')
         self.root_dir = self.mktemp(os.path.join('ubuntuonehacker', 'root_dir'))
         self.data_dir = self.mktemp('data_dir')
         self.shares_dir = self.mktemp('shares_dir')
         self.partials_dir = self.mktemp('partials_dir')
         self.main = FakeMain(self.root_dir, self.shares_dir,
                              self.data_dir, self.partials_dir)
-        self.patch(platform, 'xdg_home', self.home_dir)
 
         self.watches = set()  # keep track of added watches
 
@@ -124,10 +122,6 @@
         self.vm.log.addHandler(self.handler)
         self.addCleanup(self.vm.log.removeHandler, self.handler)
 
-        old_home = os.environ['HOME']
-        os.environ['HOME'] = self.home_dir
-        self.addCleanup(os.environ.__setitem__, 'HOME', old_home)
-
     @defer.inlineCallbacks
     def tearDown(self):
         """ cleanup main and remove the temp dir """
@@ -3742,7 +3736,6 @@
     def setUp(self):
         """Create some directories."""
         yield super(MetadataTestCase, self).setUp()
-        self.home_dir = self.mktemp('ubuntuonehacker')
         self.root_dir = self.mktemp(
             os.path.join('ubuntuonehacker', 'Ubuntu One'))
         self.data_dir = os.path.join(self.tmpdir, 'data_dir')