On 12/8/18 3:57 AM, Yasuhito FUTATSUKI wrote: > On 12/6/18 5:06 PM, Yasuhito FUTATSUKI wrote: > Jun Omae kindly reviewed and rewrote my patch to move code to check object > type into svn_swig_py_make_stream() in swigutil_py.c to minimize expansion > of typemap, and added test for parse_fns3_invalid_set_fulltext() in > swigutil_py.c which is affected by modification of > svn_swig_py_make_stream(). > (https://github.com/jun66j5/subversion/commits/improve_swig_py_stream_IF) > I also add few modification after it, so I repost modified patch. >
Most of the patch is fine. Besides some minor code formatting tweaks, there's only one bit I really changed when committing (as r1848577): > --- a/subversion/bindings/swig/python/tests/repository.py > +++ b/subversion/bindings/swig/python/tests/repository.py > @@ -231,6 +231,26 @@ class SubversionRepositoryTestCase(unittest.TestCase): > # the comparison list gets too long. > self.assertEqual(dsp.ops[:len(expected_list)], expected_list) > > + def test_parse_fns3_invalid_set_fulltext(self): > + self.cancel_calls = 0 > + def is_cancelled(): > + self.cancel_calls += 1 > + return None These mechanics around cancellation aren't necessary, and I'm guessing were just copied from other test functions nearby. I've removed them. > + class DumpStreamParserSubclass(DumpStreamParser): > + def set_fulltext(self, node_baton): > + DumpStreamParser.set_fulltext(self, node_baton) > + return 42 > + dump_path = os.path.join(os.path.dirname(sys.argv[0]), > + "trac/versioncontrol/tests/svnrepos.dump") > + stream = open(dump_path) > + try: > + dsp = DumpStreamParserSubclass() > + ptr, baton = repos.make_parse_fns3(dsp) > + self.assertRaises(TypeError, repos.parse_dumpstream3, > + stream, ptr, baton, False, is_cancelled) ...and of course, changed is_cancelled to None here.
