Noorul Islam K M <noo...@collab.net> writes:
> Philip Martin <philip.mar...@wandisco.com> writes:
>
>> Noorul Islam K M <noo...@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')
