On 2019/09/28 17:17, Yasuhito FUTATSUKI wrote:
On 2019/09/28 5:51, Johan Corveleyn wrote:
[[[
Testing Release configuration on local repository.
-- Running Swig Python tests --
..........................................................C:\Python37\lib\subprocess.py:858:
ResourceWarning: subprocess
8548 is still running
ResourceWarning, source=self)
ResourceWarning: Enable tracemalloc to get the object allocation traceback
C:\Python37\lib\unittest\case.py:628: ResourceWarning: unclosed file
<_io.BufferedReader name=3>
testMethod()
ResourceWarning: Enable tracemalloc to get the object allocation traceback
.C:\Python37\lib\unittest\case.py:628: ResourceWarning: unclosed file
<_io.BufferedReader name='R:\\temp\\tmpryeb61g1'>
testMethod()
ResourceWarning: Enable tracemalloc to get the object allocation traceback
..............................................................................................
----------------------------------------------------------------------
Ran 153 tests in 66.182s
OK
]]]
On FreeBSD those warning are also produced.
[[[
futatuki@retina-alpha[15] make check-swig-py
if [ "LD_LIBRARY_PATH" = "DYLD_LIBRARY_PATH" ]; then for d in
/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/libsvn_swig_py
/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/../../../libsvn_*; do if [ -n
"$DYLD_LIBRARY_PATH" ]; then LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$d/.libs"; else
LD_LIBRARY_PATH="$d/.libs"; fi; done; export LD_LIBRARY_PATH; fi; cd
/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python; /usr/local/bin/python3.7
/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/tests/run_all.py
........................................................../usr/local/lib/python3.7/subprocess.py:858:
ResourceWarning: subprocess 81136 is still running
ResourceWarning, source=self)
ResourceWarning: Enable tracemalloc to get the object allocation traceback
/usr/local/lib/python3.7/unittest/case.py:615: ResourceWarning: unclosed file
<_io.BufferedReader name=4>
testMethod()
ResourceWarning: Enable tracemalloc to get the object allocation traceback
./usr/local/lib/python3.7/unittest/case.py:615: ResourceWarning: unclosed file
<_io.BufferedReader name='/tmp/tmporh5038d'>
testMethod()
ResourceWarning: Enable tracemalloc to get the object allocation traceback
..............................................................................................
----------------------------------------------------------------------
Ran 153 tests in 10.773s
OK
]]]
I think these warnings comes from difference of warning between Python 2
and Python 3, and not affect quality of swig Python binding itself but
affect quality of its test code.
This was partly wrong. The warning caused by subprocess comes from
class svn.fs.FileDiff.get_pipe(), not in test code but in support class.
ResourceWarning itself appeared in Python 3.2
https://docs.python.org/3/whatsnew/3.2.html
(section "Other Language Changes")
If we can allow this warning and want to hide it, we can use
environment variable PYTHONWARNINGS='ignore::ResourceWarning::',
as the document above says.
[[[
futatuki@retina-alpha[17] env PYTHONWARNINGS='ignore::ResourceWarning::' make
check-swig-py
if [ "LD_LIBRARY_PATH" = "DYLD_LIBRARY_PATH" ]; then for d in
/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/libsvn_swig_py
/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/../../../libsvn_*; do if [ -n
"$DYLD_LIBRARY_PATH" ]; then LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$d/.libs"; else
LD_LIBRARY_PATH="$d/.libs"; fi; done; export LD_LIBRARY_PATH; fi; cd
/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python; /usr/local/bin/python3.7
/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/tests/run_all.py
.........................................................................................................................................................
----------------------------------------------------------------------
Ran 153 tests in 10.757s
OK
]]]
Of course, we can also resolve those warnings by fixing clean up code
on tests.
The patch attached is expected to fix these warnings. I already tested
on FreeBSD with Python 3.7 (treat ResourceWarning as an error).
[[[
futatuki@retina-alpha[113] env PYTHONWARNINGS='error::ResourceWarning::' make
check-swig-py
if [ "LD_LIBRARY_PATH" = "DYLD_LIBRARY_PATH" ]; then for d in
/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/libsvn_swig_py
/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/../../../libsvn_*; do if [ -n
"$DYLD_LIBRARY_PATH" ]; then LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$d/.libs"; else
LD_LIBRARY_PATH="$d/.libs"; fi; done; export LD_LIBRARY_PATH; fi; cd
/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python; /usr/local/bin/python3.7
/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/tests/run_all.py
.........................................................................................................................................................
----------------------------------------------------------------------
Ran 153 tests in 11.040s
OK
]]]
Cheers,
--
Yasuhito FUTATSUKI <futat...@poem.co.jp>
On branch swig-py3: fix resource warnings on Python 3
* subversion/bindings/swig/python/svn/fs.py
(FileDiff.procs): New list variable to hold process objects
(FileDiff.get_pipe): Hold process object to self.procs.
(FileDiff.__del__): Kill processes if they are alive before delete.
* subversion/bindings/swig/python/tests/fs.py
(SubversionFSTestCase.test_diff_repos_paths_internal,
SubversionFSTestCase.test_diff_repos_paths_external): explicitly close pipe.
Index: subversion/bindings/swig/python/svn/fs.py
===================================================================
--- subversion/bindings/swig/python/svn/fs.py (revision 1867653)
+++ subversion/bindings/swig/python/svn/fs.py (working copy)
@@ -60,6 +60,7 @@
self.tempfile1 = None
self.tempfile2 = None
self.difftemp = None
+ self.procs = []
self.root1 = root1
self.path1 = path1
@@ -124,6 +125,7 @@
# open the pipe, and return the file object for reading from the child.
p = _subprocess.Popen(cmd, stdout=_subprocess.PIPE, bufsize=-1,
close_fds=_sys.platform != "win32")
+ self.procs.append(p)
return p.stdout
else:
@@ -158,3 +160,6 @@
_os.remove(tmpfile)
except OSError:
pass
+ for proc in self.procs:
+ if proc.poll() is None:
+ proc.kill()
Index: subversion/bindings/swig/python/tests/fs.py
===================================================================
--- subversion/bindings/swig/python/tests/fs.py (revision 1867653)
+++ subversion/bindings/swig/python/tests/fs.py (working copy)
@@ -96,6 +96,7 @@
diffp = fdiff.get_pipe()
diffoutput = diffp.read().decode('utf8')
+ diffp.close()
self.assertTrue(diffoutput.find(u'-' + self.unistr) > 0)
@@ -116,6 +117,7 @@
None, None, diffoptions=[])
diffp = fdiff.get_pipe()
diffoutput = diffp.read().decode('utf8')
+ diffp.close()
self.assertTrue(diffoutput.find(u'< ' + self.unistr) > 0)
