You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Noorul Islam K M <no...@collab.net> on 2010/11/01 08:58:31 UTC
ctypes-python bindings test cases issue.
Back in August when I tried to build ctypes-python I was getting
Segmentation fault. There was an issue with ctypesgen which was causing
this. See http://code.google.com/p/ctypesgen/issues/detail?id=8
Once the issue got fixed, code generation works without any issues, but
test cases were failing with error message "revprop db already
exists". But with the recent trunk I get the following.
SubversionException: ../subversion/libsvn_repos/repos.c:1405: (apr_err=200030)
../subversion/libsvn_fs/fs-loader.c:422: (apr_err=200030)
../subversion/libsvn_fs_fs/fs.c:191: (apr_err=200030)
../subversion/libsvn_fs_fs/fs_fs.c:6522: (apr_err=200030)
../subversion/libsvn_fs_fs/fs_fs.c:1235: (apr_err=200030)
../subversion/libsvn_subr/sqlite.c:119: (apr_err=200030)
../subversion/libsvn_subr/sqlite.c:119: (apr_err=200030)
disk I/O error
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?
Thanks and Regards
Noorul
Re: ctypes-python bindings test cases issue.
Posted by Philip Martin <ph...@wandisco.com>.
Noorul Islam K M <no...@collab.net> writes:
>> I don't know how to run the ctypes tests.
>
> make check-ctypes-python
/usr/bin/python: can't open file 'none': [Errno 2] No such file or directory
sed: can't read subversion/bindings/ctypes-python/svn_all.py: No such file or directory
--
Philip
Re: ctypes-python bindings test cases issue.
Posted by Noorul Islam K M <no...@collab.net>.
Philip Martin <ph...@wandisco.com> writes:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>
>>> > 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.
>>
>> I think Philip was just enumerating all possible solutions. I think
>> making the test suite avoid the problem is the wrong solution --- it
>> would be better to cause those dangling handles to get closed at the
>> appropriate time (e.g., when there are no more references to the Python
>> repos object).
>
> It may be as simple as
>
> Index: ../src/subversion/bindings/ctypes-python/test/wc.py
> ===================================================================
> --- ../src/subversion/bindings/ctypes-python/test/wc.py (revision 1031593)
> +++ ../src/subversion/bindings/ctypes-python/test/wc.py (working copy)
> @@ -72,6 +72,7 @@
> if os.path.exists(wc_location):
> svn_io_remove_dir(wc_location, pool)
> if os.path.exists(repos_location):
> + self.repos = None
> svn_repos_delete(repos_location, pool)
> self.wc = None
>
This did not solve the problem. I still get the disk I/O error.
>
> I don't know how to run the ctypes tests.
make check-ctypes-python
Thanks and Regards
Noorul
Re: ctypes-python bindings test cases issue.
Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:
>> > 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.
>
> I think Philip was just enumerating all possible solutions. I think
> making the test suite avoid the problem is the wrong solution --- it
> would be better to cause those dangling handles to get closed at the
> appropriate time (e.g., when there are no more references to the Python
> repos object).
It may be as simple as
Index: ../src/subversion/bindings/ctypes-python/test/wc.py
===================================================================
--- ../src/subversion/bindings/ctypes-python/test/wc.py (revision 1031593)
+++ ../src/subversion/bindings/ctypes-python/test/wc.py (working copy)
@@ -72,6 +72,7 @@
if os.path.exists(wc_location):
svn_io_remove_dir(wc_location, pool)
if os.path.exists(repos_location):
+ self.repos = None
svn_repos_delete(repos_location, pool)
self.wc = None
I don't know how to run the ctypes tests.
--
Philip
Re: ctypes-python bindings test cases issue.
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Noorul Islam K M wrote on Mon, Nov 01, 2010 at 21:42:02 +0530:
> Philip Martin <ph...@wandisco.com> writes:
>
> > Noorul Islam K M <no...@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.
I think Philip was just enumerating all possible solutions. I think
making the test suite avoid the problem is the wrong solution --- it
would be better to cause those dangling handles to get closed at the
appropriate time (e.g., when there are no more references to the Python
repos object).
Re: ctypes-python bindings test cases issue.
Posted by Noorul Islam K M <no...@collab.net>.
Noorul Islam K M <no...@collab.net> writes:
> Noorul Islam K M <no...@collab.net> writes:
>
>> Noorul Islam K M <no...@collab.net> writes:
>>
>>> Philip Martin <ph...@wandisco.com> writes:
>>>
>>>> Noorul Islam K M <no...@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')
>>
>
> Are we waiting for someone to implement any other solution?
Attached is the updated patch which uses separate repository name for
each test case.
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 separate
repository location for each test case.
Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]
Philip, thank you for the review comment.
Thanks and Regards
Noorul
Re: ctypes-python bindings test cases issue.
Posted by Noorul Islam K M <no...@collab.net>.
Noorul Islam K M <no...@collab.net> writes:
> Noorul Islam K M <no...@collab.net> writes:
>
>> Philip Martin <ph...@wandisco.com> writes:
>>
>>> Noorul Islam K M <no...@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')
>
Are we waiting for someone to implement any other solution?
Thanks and Regards
Noorul
Re: ctypes-python bindings test cases issue.
Posted by Noorul Islam K M <no...@collab.net>.
Noorul Islam K M <no...@collab.net> writes:
> Philip Martin <ph...@wandisco.com> writes:
>
>> Noorul Islam K M <no...@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
Re: ctypes-python bindings test cases issue.
Posted by Noorul Islam K M <no...@collab.net>.
Philip Martin <ph...@wandisco.com> writes:
> Noorul Islam K M <no...@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
Re: ctypes-python bindings test cases issue.
Posted by Philip Martin <ph...@wandisco.com>.
Noorul Islam K M <no...@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
--
Philip