You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2011/08/08 20:32:08 UTC

svn commit: r1155044 - in /subversion/trunk/subversion/bindings/ctypes-python: csvn/repos.py csvn/wc.py test/localrepos.py test/remoterepos.py test/wc.py

Author: julianfoad
Date: Mon Aug  8 18:32:07 2011
New Revision: 1155044

URL: http://svn.apache.org/viewvc?rev=1155044&view=rev
Log:
Make the ctypes-python tests run properly by clearing the pool in which the
repository was opened between tests.  This is necessary to ensure the
rep-cache DB is closed and re-opened when the repo is deleted and re-created
at the same path.  Without that, nearly every test after the first one would
fail to initialize, saying "E160004: Youngest revision is r1, but rep-cache
contains r2", at least on some operating systems including Ubuntu Linux.

Based on a patch by Noorul, which Philip Martin pointed out to me:
<http://svn.haxx.se/dev/archive-2011-02/0702.shtml>.

* subversion/bindings/ctypes-python/csvn/repos.py
  (RemoteRepository, LocalRepository): Add a 'close' method that clears the
    pool in which the repository was opened.

* subversion/bindings/ctypes-python/csvn/wc.py
  (WC): Add a 'close' method that clears the pool in which the WC was
    opened.

* subversion/bindings/ctypes-python/test/localrepos.py
  (LocalRepositoryTestCase): Refactor the delete-from-disk code into a new
    'remove_from_disk()' method instead of calling tearDown() from within
    setUp(). In tearDown(), call the repository object's new 'close()'
    method before deleting it from disk.

* subversion/bindings/ctypes-python/test/remoterepos.py
  (RemoteRepositoryTestCase): Same.

* subversion/bindings/ctypes-python/test/wc.py
  (WCTestCase): Same, but for the WC as well as the repos.

Modified:
    subversion/trunk/subversion/bindings/ctypes-python/csvn/repos.py
    subversion/trunk/subversion/bindings/ctypes-python/csvn/wc.py
    subversion/trunk/subversion/bindings/ctypes-python/test/localrepos.py
    subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py
    subversion/trunk/subversion/bindings/ctypes-python/test/wc.py

Modified: subversion/trunk/subversion/bindings/ctypes-python/csvn/repos.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/ctypes-python/csvn/repos.py?rev=1155044&r1=1155043&r2=1155044&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/ctypes-python/csvn/repos.py (original)
+++ subversion/trunk/subversion/bindings/ctypes-python/csvn/repos.py Mon Aug  8 18:32:07 2011
@@ -95,6 +95,10 @@ class RemoteRepository(object):
         self.client[0].log_msg_baton2 = c_void_p()
         self._log_func = None
 
+    def close(self):
+        """Close this RemoteRepository object, releasing any resources."""
+        self.pool.clear()
+
     def txn(self):
         """Create a transaction"""
         return Txn(self)
@@ -396,6 +400,11 @@ class LocalRepository(object):
             svn_repos_open(byref(self._as_parameter_), path, self.pool)
         self.fs = _fs(self)
 
+    def close(self):
+        """Close this LocalRepository object, releasing any resources. In
+           particular, this closes the rep-cache DB."""
+        self.pool.clear()
+
     def latest_revnum(self):
         """Get the latest revision in the repository"""
         return self.fs.latest_revnum()

Modified: subversion/trunk/subversion/bindings/ctypes-python/csvn/wc.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/ctypes-python/csvn/wc.py?rev=1155044&r1=1155043&r2=1155044&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/ctypes-python/csvn/wc.py (original)
+++ subversion/trunk/subversion/bindings/ctypes-python/csvn/wc.py Mon Aug  8 18:32:07 2011
@@ -79,6 +79,10 @@ class WC(object):
         self.client[0].log_msg_baton2 = c_void_p()
         self._log_func = None
 
+    def close(self):
+        """Close this WC object, releasing any resources."""
+        self.pool.clear()
+
     def copy(self, src, dest, rev = ""):
         """Copy src to dest.
 

Modified: subversion/trunk/subversion/bindings/ctypes-python/test/localrepos.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/ctypes-python/test/localrepos.py?rev=1155044&r1=1155043&r2=1155044&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/ctypes-python/test/localrepos.py (original)
+++ subversion/trunk/subversion/bindings/ctypes-python/test/localrepos.py Mon Aug  8 18:32:07 2011
@@ -33,15 +33,21 @@ class LocalRepositoryTestCase(unittest.T
         dumpfile = open(os.path.join(os.path.split(__file__)[0],
                         'test.dumpfile'))
 
-        # Just in case a preivous test instance was not properly cleaned up
-        self.tearDown()
+        # Just in case a previous test instance was not properly cleaned up
+        self.remove_from_disk()
+
         self.repos = LocalRepository(repos_location, create=True)
         self.repos.load(dumpfile)
 
     def tearDown(self):
+        self.repos.close()
+        self.remove_from_disk()
+        self.repos = None
+
+    def remove_from_disk(self):
+        """Remove anything left on disk"""
         if os.path.exists(repos_location):
             svn_repos_delete(repos_location, Pool())
-        self.repos = None
 
     def test_local_latest_revnum(self):
         self.assertEqual(9, self.repos.latest_revnum())

Modified: subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py?rev=1155044&r1=1155043&r2=1155044&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py (original)
+++ subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py Mon Aug  8 18:32:07 2011
@@ -46,17 +46,24 @@ class RemoteRepositoryTestCase(unittest.
         dumpfile = open(os.path.join(os.path.split(__file__)[0],
                         'test.dumpfile'))
 
-        # Just in case a preivous test instance was not properly cleaned up
-        self.tearDown()
+        # Just in case a previous test instance was not properly cleaned up
+        self.remove_from_disk()
+
         self.repos = LocalRepository(repos_location, create=True)
         self.repos.load(dumpfile)
+        self.repos.close()
 
         self.repos = RemoteRepository(repos_url)
 
     def tearDown(self):
+        self.repos.close()
+        self.remove_from_disk()
+        self.repos = None
+
+    def remove_from_disk(self):
+        """Remove anything left on disk"""
         if os.path.exists(repos_location):
             svn_repos_delete(repos_location, Pool())
-        self.repos = None
 
     def test_remote_latest_revnum(self):
         self.assertEqual(9, self.repos.latest_revnum())

Modified: subversion/trunk/subversion/bindings/ctypes-python/test/wc.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/ctypes-python/test/wc.py?rev=1155044&r1=1155043&r2=1155044&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/ctypes-python/test/wc.py (original)
+++ subversion/trunk/subversion/bindings/ctypes-python/test/wc.py Mon Aug  8 18:32:07 2011
@@ -59,8 +59,9 @@ class WCTestCase(unittest.TestCase):
         dumpfile = open(os.path.join(os.path.split(__file__)[0],
                         'test.dumpfile'))
 
-        # Just in case a preivous test instance was not properly cleaned up
-        self.tearDown()
+        # Just in case a previous test instance was not properly cleaned up
+        self.remove_from_disk()
+
         self.repos = LocalRepository(repos_location, create=True)
         self.repos.load(dumpfile)
 
@@ -68,12 +69,18 @@ class WCTestCase(unittest.TestCase):
         self.wc.checkout(repo_url)
 
     def tearDown(self):
+        self.repos.close()
+        self.wc.close()
+        self.remove_from_disk()
+        self.wc = None
+
+    def remove_from_disk(self):
+        """Remove anything left on disk"""
         pool = Pool()
         if os.path.exists(wc_location):
             svn_io_remove_dir(wc_location, pool)
         if os.path.exists(repos_location):
             svn_repos_delete(repos_location, pool)
-        self.wc = None
 
     def _info_receiver(self, path, info):
         self.last_info = info



Re: svn commit: r1155044 - in /subversion/trunk/subversion/bindings/ctypes-python: csvn/repos.py csvn/wc.py test/localrepos.py test/remoterepos.py test/wc.py

Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2011-08-08 at 21:44 +0300, Daniel Shahaf wrote:
> julianfoad@apache.org wrote on Mon, Aug 08, 2011 at 18:32:08 -0000:
> > +++ subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py Mon Aug  8 18:32:07 2011
> > @@ -46,17 +46,24 @@ class RemoteRepositoryTestCase(unittest.
> >          dumpfile = open(os.path.join(os.path.split(__file__)[0],
> >                          'test.dumpfile'))
> >  
> > -        # Just in case a preivous test instance was not properly cleaned up
> > -        self.tearDown()
> > +        # Just in case a previous test instance was not properly cleaned up
> > +        self.remove_from_disk()
> > +
> >          self.repos = LocalRepository(repos_location, create=True)
> >          self.repos.load(dumpfile)
> > +        self.repos.close()
> >  
> 
> Instead of an explicit .close(), could you rely on the object's cleanup
> method (reverse of __init__) cleaning it, when the last reference to
> self.repos is removed on the next line?

I'd like to do that, and I tried, defining a '__del__' method, but I
couldn't seem to get it to be called.

- Julian


> +1 on the rest of the patch.
> 
> >          self.repos = RemoteRepository(repos_url)



Re: svn commit: r1155044 - in /subversion/trunk/subversion/bindings/ctypes-python: csvn/repos.py csvn/wc.py test/localrepos.py test/remoterepos.py test/wc.py

Posted by Julian Foad <ju...@wandisco.com>.
On Mon, 2011-08-08 at 21:44 +0300, Daniel Shahaf wrote:
> julianfoad@apache.org wrote on Mon, Aug 08, 2011 at 18:32:08 -0000:
> > +++ subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py Mon Aug  8 18:32:07 2011
> > @@ -46,17 +46,24 @@ class RemoteRepositoryTestCase(unittest.
> >          dumpfile = open(os.path.join(os.path.split(__file__)[0],
> >                          'test.dumpfile'))
> >  
> > -        # Just in case a preivous test instance was not properly cleaned up
> > -        self.tearDown()
> > +        # Just in case a previous test instance was not properly cleaned up
> > +        self.remove_from_disk()
> > +
> >          self.repos = LocalRepository(repos_location, create=True)
> >          self.repos.load(dumpfile)
> > +        self.repos.close()
> >  
> 
> Instead of an explicit .close(), could you rely on the object's cleanup
> method (reverse of __init__) cleaning it, when the last reference to
> self.repos is removed on the next line?

I'd like to do that, and I tried, defining a '__del__' method, but I
couldn't seem to get it to be called.

- Julian


> +1 on the rest of the patch.
> 
> >          self.repos = RemoteRepository(repos_url)



Re: svn commit: r1155044 - in /subversion/trunk/subversion/bindings/ctypes-python: csvn/repos.py csvn/wc.py test/localrepos.py test/remoterepos.py test/wc.py

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
julianfoad@apache.org wrote on Mon, Aug 08, 2011 at 18:32:08 -0000:
> +++ subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py Mon Aug  8 18:32:07 2011
> @@ -46,17 +46,24 @@ class RemoteRepositoryTestCase(unittest.
>          dumpfile = open(os.path.join(os.path.split(__file__)[0],
>                          'test.dumpfile'))
>  
> -        # Just in case a preivous test instance was not properly cleaned up
> -        self.tearDown()
> +        # Just in case a previous test instance was not properly cleaned up
> +        self.remove_from_disk()
> +
>          self.repos = LocalRepository(repos_location, create=True)
>          self.repos.load(dumpfile)
> +        self.repos.close()
>  

Instead of an explicit .close(), could you rely on the object's cleanup
method (reverse of __init__) cleaning it, when the last reference to
self.repos is removed on the next line?

+1 on the rest of the patch.

>          self.repos = RemoteRepository(repos_url)

Re: svn commit: r1155044 - in /subversion/trunk/subversion/bindings/ctypes-python: csvn/repos.py csvn/wc.py test/localrepos.py test/remoterepos.py test/wc.py

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
julianfoad@apache.org wrote on Mon, Aug 08, 2011 at 18:32:08 -0000:
> +++ subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py Mon Aug  8 18:32:07 2011
> @@ -46,17 +46,24 @@ class RemoteRepositoryTestCase(unittest.
>          dumpfile = open(os.path.join(os.path.split(__file__)[0],
>                          'test.dumpfile'))
>  
> -        # Just in case a preivous test instance was not properly cleaned up
> -        self.tearDown()
> +        # Just in case a previous test instance was not properly cleaned up
> +        self.remove_from_disk()
> +
>          self.repos = LocalRepository(repos_location, create=True)
>          self.repos.load(dumpfile)
> +        self.repos.close()
>  

Instead of an explicit .close(), could you rely on the object's cleanup
method (reverse of __init__) cleaning it, when the last reference to
self.repos is removed on the next line?

+1 on the rest of the patch.

>          self.repos = RemoteRepository(repos_url)