[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: ctypes-python bindings test cases issue.

From: Noorul Islam K M <noorul_at_collab.net>
Date: Mon, 01 Nov 2010 21:46:33 +0530

Noorul Islam K M <noorul_at_collab.net> writes:

> Philip Martin <philip.martin_at_wandisco.com> writes:
>
>> Noorul Islam K M <noorul_at_collab.net> writes:
>>
>>> Test cases are written using python unittest framework and it has two
>>> methods, setUp() and tearDown() which gets executed for every case. In
>>> tearDown(), repository which is created in setUp() is deleted using
>>> svn_repos_delete(). During first iteration there are no issues but in
>>> the second iteration (test case), the system throws the above mentioned
>>> error. Using lsof command I could see something like this
>>>
>>> python 18111 noorul 4u REG 8,1 5120 279333 /tmp/svn_test_repos/db/revp
>>> rops/revprops.db (deleted)
>>>
>>> Does this mean that the sqlite file pointers are not completely
>>> destroyed?
>>
>> Yes. The repository handle from the previous svn_repos_create function
>> is still around when svn_repos_delete is called.
>>
>> This is a new problem caused by the revprop packing. Also, there
>> doesn't appear to be an API for explicitly closing the repository
>> handle. Solutions include:
>>
>> - adding an svn_repos_close API
>> - clearing or destroying the pool passed to svn_repos_create
>> - having the test create repositories at different locations
>
> I have implemented the last solution and attached is the patch for the
> same.
>
> Log
>
> [[[
>
> Modify test cases to overcome limitations in closing repository handle.
>
> * subversion/bindings/ctypes-python/test/wc.py:
> * subversion/bindings/ctypes-python/test/localrepos.py:
> * subversion/bindings/ctypes-python/test/remoterepos.py: Use random
> repository location during each iteration.
>
> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
>
> ]]]
>
> Thanks and Regards
> Noorul
>
> Index: build/run_ctypesgen.sh
> ===================================================================
> --- build/run_ctypesgen.sh (revision 1028789)
> +++ build/run_ctypesgen.sh (working copy)
> @@ -86,4 +86,4 @@
> (cat $abs_srcdir/$cp_relpath/csvn/core/functions.py.in; \
> sed -e '/^FILE =/d' $output | \
> perl -pe 's{(\s+\w+)\.restype = POINTER\(svn_error_t\)}{\1.restype = POINTER(svn_error_t)\n\1.errcheck = _svn_errcheck}' \
> - ) > $abs_builddir/$cp_relpath/csvn/core/functions.py
> + ) > $abs_srcdir/$cp_relpath/csvn/core/functions.py

This patch should not include build/run_ctypesgen.sh file. Sorry for the
mess. Attached is the revised one.

Thanks and Regards
Noorul

Index: subversion/bindings/ctypes-python/test/wc.py
===================================================================
--- subversion/bindings/ctypes-python/test/wc.py (revision 1028789)
+++ subversion/bindings/ctypes-python/test/wc.py (working copy)
@@ -22,6 +22,7 @@
 import os
 import shutil
 import tempfile
+import random
 from sys import version_info # For Python version check
 if version_info[0] >= 3:
   # Python >=3.0
@@ -36,22 +37,6 @@
 
 locale.setlocale(locale.LC_ALL, "C")
 
-repos_location = os.path.join(tempfile.gettempdir(), "svn_test_repos")
-wc_location = os.path.join(tempfile.gettempdir(), "svn_test_wc")
-repo_url = pathname2url(repos_location)
-if repo_url.startswith("///"):
- # Don't add extra slashes if they're already present.
- # (This is important for Windows compatibility).
- repo_url = "file:" + repo_url
-else:
- # If the URL simply starts with '/', we need to add two
- # extra slashes to make it a valid 'file://' URL
- repo_url = "file://" + repo_url
-
-if os.sep != "/":
- repos_location = repos_location.replace(os.sep, "/")
- wc_location = wc_location.replace(os.sep, "/")
-
 class WCTestCase(unittest.TestCase):
     """Test case for Subversion WC layer."""
 
@@ -59,20 +44,24 @@
         dumpfile = open(os.path.join(os.path.split(__file__)[0],
                         'test.dumpfile'))
 
+ self.repos_location = self.get_repos_location()
+ self.repos_url = self.get_repos_url(self.repos_location)
+ self.wc_location = self.get_wc_location()
+
         # Just in case a preivous test instance was not properly cleaned up
         self.tearDown()
- self.repos = LocalRepository(repos_location, create=True)
+ self.repos = LocalRepository(self.repos_location, create=True)
         self.repos.load(dumpfile)
 
- self.wc = WC(wc_location)
- self.wc.checkout(repo_url)
+ self.wc = WC(self.wc_location)
+ self.wc.checkout(self.repos_url)
 
     def tearDown(self):
         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)
+ if os.path.exists(self.wc_location):
+ svn_io_remove_dir(self.wc_location, pool)
+ if os.path.exists(self.repos_location):
+ svn_repos_delete(self.repos_location, pool)
         self.wc = None
 
     def _info_receiver(self, path, info):
@@ -82,7 +71,7 @@
         self.wc.info(path="trunk/README.txt",info_func=self._info_receiver)
         self.assertEqual(9, self.last_info.rev)
         self.assertEqual(svn_node_file, self.last_info.kind)
- self.assertEqual(repo_url, self.last_info.repos_root_URL)
+ self.assertEqual(self.repos_url, self.last_info.repos_root_URL)
         self.assertEqual("890f2569-e600-4cfc-842a-f574dec58d87",
             self.last_info.repos_UUID)
         self.assertEqual(9, self.last_info.last_changed_rev)
@@ -118,7 +107,7 @@
         self.assertEqual(svn_wc_schedule_add, self.last_info.schedule)
 
     def test_add(self):
- f = open("%s/trunk/ADDED.txt" % wc_location, "w")
+ f = open("%s/trunk/ADDED.txt" % self.wc_location, "w")
         f.write("Something")
         f.close()
 
@@ -134,7 +123,7 @@
         self.assertEqual(svn_wc_schedule_normal, self.last_info.schedule)
 
     def test_diff(self):
- path = "%s/trunk/README.txt" % wc_location
+ path = "%s/trunk/README.txt" % self.wc_location
 
         diffstring="""Index: """+path+"""
 ===================================================================
@@ -158,7 +147,7 @@
         diffresult = difffile.read().replace("\r","")
         self.assertEqual(diffstring, diffresult)
 
- path = "%s/branches/0.x/README.txt" % wc_location
+ path = "%s/branches/0.x/README.txt" % self.wc_location
         diffstring="""Index: """+path+"""
 ===================================================================
 --- """+path+"""\t(revision 0)
@@ -191,14 +180,14 @@
 
     def test_propget(self):
         props = self.wc.propget("Awesome")
- path = "%s/trunk/README.txt" % wc_location
+ path = "%s/trunk/README.txt" % self.wc_location
         if not path in props.keys():
             self.fail("File missing in propget")
 
     def test_propset(self):
         self.wc.propset("testprop", "testval", "branches/0.x/README.txt")
         props = self.wc.propget("testprop", "branches/0.x/README.txt")
- if not "%s/branches/0.x/README.txt" % wc_location in \
+ if not "%s/branches/0.x/README.txt" % self.wc_location in \
                 props.keys():
 
             self.fail("Property not set")
@@ -208,7 +197,7 @@
         results = self.wc.update([path], revnum=7)
         self.assertEqual(results[0], 7)
         props = self.wc.propget("Awesome")
- if "%s/%s" % (wc_location, path) in \
+ if "%s/%s" % (self.wc_location, path) in \
                 props.keys():
             self.fail("File not updated to old revision")
         results = self.wc.update([path])
@@ -216,12 +205,12 @@
         self.test_propget()
 
     def test_switch(self):
- self.wc.switch("trunk", "%s/tags" % repo_url)
- if os.path.exists("%s/trunk/README.txt" % wc_location):
+ self.wc.switch("trunk", "%s/tags" % self.repos_url)
+ if os.path.exists("%s/trunk/README.txt" % self.wc_location):
             self.fail("Switch did not happen")
 
     def test_lock(self):
- self.wc.lock(["%s/trunk/README.txt" % wc_location],
+ self.wc.lock(["%s/trunk/README.txt" % self.wc_location],
                         "Test lock")
         self.wc.info(path="trunk/README.txt",
             info_func=self._info_receiver)
@@ -229,7 +218,7 @@
             self.fail("Lock not aquired")
 
     def test_unlock(self):
- path = "%s/trunk/README.txt" % wc_location
+ path = "%s/trunk/README.txt" % self.wc_location
         self.wc.lock([path], "Test lock")
         self.wc.info(path=path,
             info_func=self._info_receiver)
@@ -242,7 +231,29 @@
         if self.last_info.lock:
             self.fail("Lock not released")
 
+ @staticmethod
+ def get_repos_location():
+ return os.path.join(tempfile.gettempdir(),
+ "svn_test_repos_%f" % random.random())
 
+ @staticmethod
+ def get_repos_url(repos_location):
+ repos_url = pathname2url(repos_location)
+ if repos_url.startswith("///"):
+ # Don't add extra slashes if they're already present.
+ # (This is important for Windows compatibility).
+ repos_url = "file:" + repos_url
+ else:
+ # If the URL simply starts with '/', we need to add two
+ # extra slashes to make it a valid 'file://' URL
+ repos_url = "file://" + repos_url
+ return repos_url
+
+ @staticmethod
+ def get_wc_location():
+ return os.path.join(tempfile.gettempdir(),
+ "svn_test_wc_%f" % random.random())
+
 def suite():
     return unittest.makeSuite(WCTestCase, 'test')
 
Index: subversion/bindings/ctypes-python/test/localrepos.py
===================================================================
--- subversion/bindings/ctypes-python/test/localrepos.py (revision 1028789)
+++ subversion/bindings/ctypes-python/test/localrepos.py (working copy)
@@ -21,26 +21,26 @@
 import os
 import shutil
 import tempfile
+import random
 from csvn.core import *
 from urllib import pathname2url
 from csvn.repos import LocalRepository, RemoteRepository
 
-repos_location = os.path.join(tempfile.gettempdir(), "svn_test_repos")
-
 class LocalRepositoryTestCase(unittest.TestCase):
 
     def setUp(self):
+ self.repos_location = self.get_repos_location()
         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()
- self.repos = LocalRepository(repos_location, create=True)
+ self.repos = LocalRepository(self.repos_location, create=True)
         self.repos.load(dumpfile)
 
     def tearDown(self):
- if os.path.exists(repos_location):
- svn_repos_delete(repos_location, Pool())
+ if os.path.exists(self.repos_location):
+ svn_repos_delete(self.repos_location, Pool())
         self.repos = None
 
     def test_local_latest_revnum(self):
@@ -62,6 +62,11 @@
         self.assertEqual(svn_node_none,
             self.repos.check_path("does_not_compute"))
 
+ @staticmethod
+ def get_repos_location():
+ return os.path.join(tempfile.gettempdir(),
+ "svn_test_repos_%f" % random.random())
+
 def suite():
     return unittest.makeSuite(LocalRepositoryTestCase, 'test')
 
Index: subversion/bindings/ctypes-python/test/remoterepos.py
===================================================================
--- subversion/bindings/ctypes-python/test/remoterepos.py (revision 1028789)
+++ subversion/bindings/ctypes-python/test/remoterepos.py (working copy)
@@ -23,39 +23,31 @@
 import shutil
 import tempfile
 import sys
+import random
 from csvn.core import *
 from urllib import pathname2url
 from csvn.repos import LocalRepository, RemoteRepository
 from stat import *
 
-repos_location = os.path.join(tempfile.gettempdir(), "svn_test_repos")
-repos_url = pathname2url(repos_location)
-if repos_url.startswith("///"):
- # Don't add extra slashes if they're already present.
- # (This is important for Windows compatibility).
- repos_url = "file:" + repos_url
-else:
- # If the URL simply starts with '/', we need to add two
- # extra slashes to make it a valid 'file://' URL
- repos_url = "file://" + repos_url
-
-
 class RemoteRepositoryTestCase(unittest.TestCase):
 
     def setUp(self):
         dumpfile = open(os.path.join(os.path.split(__file__)[0],
                         'test.dumpfile'))
 
+ self.repos_location = self.get_repos_location()
+ self.repos_url = self.get_repos_url(self.repos_location)
+
         # Just in case a preivous test instance was not properly cleaned up
         self.tearDown()
- self.repos = LocalRepository(repos_location, create=True)
+ self.repos = LocalRepository(self.repos_location, create=True)
         self.repos.load(dumpfile)
 
- self.repos = RemoteRepository(repos_url)
+ self.repos = RemoteRepository(self.repos_url)
 
     def tearDown(self):
- if os.path.exists(repos_location):
- svn_repos_delete(repos_location, Pool())
+ if os.path.exists(self.repos_location):
+ svn_repos_delete(self.repos_location, Pool())
         self.repos = None
 
     def test_remote_latest_revnum(self):
@@ -98,12 +90,12 @@
         # For revprops to be changeable, we need to have a hook.
         # We'll make a hook that accepts anything
         if sys.platform == "win32":
- hook = os.path.join(repos_location, "hooks", "pre-revprop-change.bat")
+ hook = os.path.join(self.repos_location, "hooks", "pre-revprop-change.bat")
             f = open(hook, "w")
             f.write("@exit")
             f.close()
         else:
- hook = os.path.join(repos_location, "hooks", "pre-revprop-change")
+ hook = os.path.join(self.repos_location, "hooks", "pre-revprop-change")
             f = open(hook, "w")
             f.write("#!/bin/sh\nexit 0;")
             f.close()
@@ -133,9 +125,26 @@
         f = open(newfile, "w")
         f.write("Some new stuff\n")
         f.close()
- commit_info = self.repos.svnimport(newfile, "%s/newfile.txt" % repos_url, log_func=self._log_func)
+ commit_info = self.repos.svnimport(newfile, "%s/newfile.txt" % self.repos_url, log_func=self._log_func)
         self.assertEqual(commit_info.revision, 10)
 
+ @staticmethod
+ def get_repos_location():
+ return os.path.join(tempfile.gettempdir(),
+ "svn_test_repos_%f" % random.random())
+ @staticmethod
+ def get_repos_url(repos_location):
+ repos_url = pathname2url(repos_location)
+ if repos_url.startswith("///"):
+ # Don't add extra slashes if they're already present.
+ # (This is important for Windows compatibility).
+ repos_url = "file:" + repos_url
+ else:
+ # If the URL simply starts with '/', we need to add two
+ # extra slashes to make it a valid 'file://' URL
+ repos_url = "file://" + repos_url
+ return repos_url
+
 def suite():
     return unittest.makeSuite(RemoteRepositoryTestCase, 'test')
 
Received on 2010-11-01 17:17:31 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.