Re: svn commit: r1849778 - in /subversion/branches/swig-py3/subversion/bindings/swig: ./ include/ python/libsvn_swig_py/ python/tests/

[Daniel Shahaf <d....@daniel.shahaf.name>]

troycurtisjr@apache.org wrote on Wed, Dec 26, 2018 at 21:07:53 -0000:
> On branch swig-py3: Complete Python SWIG bindings for svn_client.h.
> 
> * subversion/bindings/swig/python/tests/client.py
>   (SubversionClientTestCase.assert_all_instances_of): New helper method.
>   (SubversionClientTestCase.test_platform_providers): Add usage of
>     assert_all_instances_of() helper method.
>   (SubversionClientTestCase.test_conflict): New test method.
>   (SubversionClientTestCase.test_shelf): New test method.
> 
> +++ subversion/branches/swig-py3/subversion/bindings/swig/python/tests/client.py Wed Dec 26 21:07:53 2018
> @@ -32,6 +32,11 @@ except ImportError:
>  class SubversionClientTestCase(unittest.TestCase):
>    """Test cases for the basic SWIG Subversion client layer"""
>  
> +  def assert_all_instances_of(self, iterable, instancetype):
> +    """"Asserts that all object from iterable are an instance of instancetype."""
> +
> +    self.assertTrue(not [x for x in iterable if not isinstance(x, instancetype)])

This is correct but hard to read and isn't going to generate usable
tracebacks.  How about:

    for x in iterable:
        self.assertIsInstance(x, instance_type)

> @@ -497,6 +502,87 @@ class SubversionClientTestCase(unittest.
>      expected_paths.sort()
>      self.assertEqual(self.notified_paths, expected_paths)
>  
> +  def test_conflict(self):
> +    """Test conflict api."""
⋮
> +    self.assertTrue(isinstance(conflict, client.svn_client_conflict_t))

Use self.assertIsInstance, again to have usable tracebacks?  It's
available in py2.7 which is the oldest we support.  (A few more
instances of this later in the diff)

> +    conflict_opts = client.conflict_tree_get_resolution_options(conflict, self.client_ctx)
> +
> +    self.assertTrue(isinstance(conflict_opts, list))
> +    self.assert_all_instances_of(conflict_opts, client.svn_client_conflict_option_t)
> +
> +    pool.clear()

Cheers,

Daniel