You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Madan U Sreenivasan <ma...@collab.net> on 2006/06/17 09:24:18 UTC

[PATCH] Improve test cases for svn.wc python bindings

Hi,

   Pl. find attached some improved tests for svn.wc python bindings.

   This patch is dependant on the import of shutil module which is also  
present in a patch I submitted earlier (but not committed, yet)...
        http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=116821
   So, that might have to be taken care of.

   Pl. note that there are two failing test cases, which are supposed to  
work as per the svn_wc.h documentation. so, I have kept them as-is (added  
comments tho). Am trying to figure out the best way to fix these  
issues/documentation and then would come back and fix these failures.

[[[
Improve existing tests for the svn.wc python binding.

* subversion/bindings/swig/python/tests/wc.py
   (global): import shutil.
   (test_check_wc): Add invalid-file case.
   (test_get_ancestry): Add invalid-file case.
   (test_status): Add cases with varying input.
   (test_is_normal_prop): Use failUnless instead of assert_.
   (test_is_wc_prop): Use failUnless instead of assert_.
   (test_is_entry_prop): Use failUnless instead of assert_.
   (test_get_pristine_copy_path): Add test for removal of text-base case.
]]]

Regards,
Madan.

Re: [PATCH] Improve test cases for svn.wc python bindings

Posted by Daniel Rall <dl...@collab.net>.
It doesn't appear that the entirety of this patch was committed to the
Python bindings.  What is it's status?

- Dan

On Fri, 23 Jun 2006, Madan S. wrote:

> Hi,
> 
> On Fri, 23 Jun 2006 15:54:52 +0530, Jelmer Vernooij <je...@samba.org>  
> wrote:
> 
> [snip]
> >Sorry, I've been busy with other things. I'll try to have a look at your
> >patch over this weekend.
> 
> I have modified the patch as per your comments on irc (retain assert_ for  
> existing tests). Pl. find attached.
> There is a line in the setUp() function where the change is the same as in  
> my other patch at  
> http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=117063. FYI.
> 
> [[[
> Improve existing tests for the svn.wc python binding.
> 
> * subversion/bindings/swig/python/tests/wc.py
>   (setUp): Modify adm_open3() call to open the whole working-copy tree.
>   (test_check_wc): Add invalid-file case.
>   (test_get_ancestry): Add invalid-file case.
>   (test_status): Add cases with varying input.
> ]]]
> 
> Regards,
> Madan.

> Index: subversion/bindings/swig/python/tests/wc.py
> ===================================================================
> --- subversion/bindings/swig/python/tests/wc.py	(revision 20225)
> +++ subversion/bindings/swig/python/tests/wc.py	(working copy)
> @@ -31,7 +31,7 @@
>      client.checkout2(self.repos_url, self.path, rev, rev, True, True, 
>              client_ctx)
>  
> -    self.wc = wc.adm_open3(None, self.path, True, 0, None)
> +    self.wc = wc.adm_open3(None, self.path, True, -1, None)
>  
>    def test_entry(self):
>        wc_entry = wc.entry(self.path, self.wc, True)
> @@ -75,14 +75,34 @@
>  
>    def test_check_wc(self):
>        self.assert_(wc.check_wc(self.path) > 0)
> +      self.assertRaises(SubversionException, wc.check_wc,
> +                        os.path.join(self.path,"NONEXISTANTFILE"))
>  
>    def test_get_ancestry(self):
>        self.assertEqual([self.repos_url, 12], 
>                         wc.get_ancestry(self.path, self.wc))
> +      self.assertRaises(SubversionException,
> +                        wc.get_ancestry,
> +                        os.path.join(self.path, "NONEXISTANTFILE"),
> +                        self.wc)
>  
>    def test_status(self):
>        wc.status2(self.path, self.wc)
>  
> +      # Prepare for the tests: Remove a versioned file, add an unversioned file
> +      removed_versioned_file = os.path.join(self.path, "trunk", "README.txt")
> +      unversioned_file = os.path.join(self.path, "UNVERSIONEDFILE")
> +      nonexistant_file = os.path.join(self.path, "NONEXISTANTFILE")
> +      os.remove(removed_versioned_file)
> +      open(unversioned_file, 'w').close()
> +
> +      self.assertEqual(wc.status2(removed_versioned_file, self.wc).text_status,
> +                       wc.svn_wc_status_missing)
> +      self.assertEqual(wc.status2(nonexistant_file, self.wc).text_status,
> +                       wc.svn_wc_status_none)
> +      self.assertEqual(wc.status2(unversioned_file, self.wc).text_status,
> +                       wc.svn_wc_status_unversioned)
> +
>    def test_is_normal_prop(self):
>        self.failIf(wc.is_normal_prop('svn:wc:foo:bar'))
>        self.failIf(wc.is_normal_prop('svn:entry:foo:bar'))

> Improve existing tests for the svn.wc python binding.
> 
> * subversion/bindings/swig/python/tests/wc.py
>   (setUp): Modify adm_open3() call to open the whole working-copy tree.
>   (test_check_wc): Add invalid-file case.
>   (test_get_ancestry): Add invalid-file case.
>   (test_status): Add cases with varying input.

Re: [PATCH] Improve test cases for svn.wc python bindings

Posted by Madan U Sreenivasan <ma...@collab.net>.
Hi,

On Fri, 23 Jun 2006 15:54:52 +0530, Jelmer Vernooij <je...@samba.org>  
wrote:

[snip]
> Sorry, I've been busy with other things. I'll try to have a look at your
> patch over this weekend.

I have modified the patch as per your comments on irc (retain assert_ for  
existing tests). Pl. find attached.
There is a line in the setUp() function where the change is the same as in  
my other patch at  
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=117063. FYI.

[[[
Improve existing tests for the svn.wc python binding.

* subversion/bindings/swig/python/tests/wc.py
   (setUp): Modify adm_open3() call to open the whole working-copy tree.
   (test_check_wc): Add invalid-file case.
   (test_get_ancestry): Add invalid-file case.
   (test_status): Add cases with varying input.
]]]

Regards,
Madan.

Re: [PATCH] Improve test cases for svn.wc python bindings

Posted by Jelmer Vernooij <je...@samba.org>.
On Thu, 2006-06-22 at 17:31 +0530, Madan U Sreenivasan wrote:
> On Mon, 19 Jun 2006 14:50:34 +0530, Madan U Sreenivasan <ma...@collab.net>  
> wrote:
> 
> > On Sat, 17 Jun 2006 17:38:01 +0530, Madan U Sreenivasan  
> > <ma...@collab.net> wrote:
> >
> >> On Sat, 17 Jun 2006 15:25:35 +0530, Jelmer Vernooij <je...@samba.org>  
> >> wrote:
> > [snip]
> >>> Looks good! Two small comments from a quick glance over your patch:
> >>> Why the assert_ -> failUnless change?
> >>
> >> IIUC, assert_ will cause and assertion and hence an error. failUnless  
> >> will cause a failure (test failure)
> >
> > No, I was wrong, assert_() and failUnless() effectively do the same. But  
> > I still feel that the name `failUnless' makes more sense in a unit-test  
> > scenario. What do you think?
> >
> >>> I'm not sure whether adding tests that fail are a good idea - it makes
> >>> it harder to catch real regressions. Can you comment out the failing
> >>> ones and a TODO or send a fix along that fixes the actual bug?
> >>
> >> Good idea. Will do that, and send the patch again. Thanks. :)
> >
> > Jelmer: I have this patch ready... will resend this patch, after getting  
> > done, what you have to say about assert_() vs failUnless(). Hope this is  
> > okay.
> 
> Jelmer...?
Sorry, I've been busy with other things. I'll try to have a look at your
patch over this weekend.

Cheers,

Jelmer
-- 
Jelmer Vernooij <je...@samba.org> - http://samba.org/~jelmer/

Re: [PATCH] Improve test cases for svn.wc python bindings

Posted by Madan U Sreenivasan <ma...@collab.net>.
On Mon, 19 Jun 2006 14:50:34 +0530, Madan U Sreenivasan <ma...@collab.net>  
wrote:

> On Sat, 17 Jun 2006 17:38:01 +0530, Madan U Sreenivasan  
> <ma...@collab.net> wrote:
>
>> On Sat, 17 Jun 2006 15:25:35 +0530, Jelmer Vernooij <je...@samba.org>  
>> wrote:
> [snip]
>>> Looks good! Two small comments from a quick glance over your patch:
>>> Why the assert_ -> failUnless change?
>>
>> IIUC, assert_ will cause and assertion and hence an error. failUnless  
>> will cause a failure (test failure)
>
> No, I was wrong, assert_() and failUnless() effectively do the same. But  
> I still feel that the name `failUnless' makes more sense in a unit-test  
> scenario. What do you think?
>
>>> I'm not sure whether adding tests that fail are a good idea - it makes
>>> it harder to catch real regressions. Can you comment out the failing
>>> ones and a TODO or send a fix along that fixes the actual bug?
>>
>> Good idea. Will do that, and send the patch again. Thanks. :)
>
> Jelmer: I have this patch ready... will resend this patch, after getting  
> done, what you have to say about assert_() vs failUnless(). Hope this is  
> okay.

Jelmer...?


Regards,
Madan.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Improve test cases for svn.wc python bindings

Posted by Madan U Sreenivasan <ma...@collab.net>.
On Sat, 17 Jun 2006 17:38:01 +0530, Madan U Sreenivasan <ma...@collab.net>  
wrote:

> On Sat, 17 Jun 2006 15:25:35 +0530, Jelmer Vernooij <je...@samba.org>  
> wrote:
[snip]
>> Looks good! Two small comments from a quick glance over your patch:
>> Why the assert_ -> failUnless change?
>
> IIUC, assert_ will cause and assertion and hence an error. failUnless  
> will cause a failure (test failure)

No, I was wrong, assert_() and failUnless() effectively do the same. But I  
still feel that the name `failUnless' makes more sense in a unit-test  
scenario. What do you think?

>> I'm not sure whether adding tests that fail are a good idea - it makes
>> it harder to catch real regressions. Can you comment out the failing
>> ones and a TODO or send a fix along that fixes the actual bug?
>
> Good idea. Will do that, and send the patch again. Thanks. :)

Jelmer: I have this patch ready... will resend this patch, after getting  
done, what you have to say about assert_() vs failUnless(). Hope this is  
okay.

Regards,
Madan.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Improve test cases for svn.wc python bindings

Posted by Madan U Sreenivasan <ma...@collab.net>.
On Sat, 17 Jun 2006 15:25:35 +0530, Jelmer Vernooij <je...@samba.org>  
wrote:

> Hi Madan,
>
> On Sat, 2006-06-17 at 14:54 +0530, Madan U Sreenivasan wrote:
>>    Pl. find attached some improved tests for svn.wc python bindings.
>>
>>    This patch is dependant on the import of shutil module which is also
>> present in a patch I submitted earlier (but not committed, yet)...
>>         http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=116821
> I'll try to have a look at it later today (if nobody has committed it
> yet by then)
>
>>    Pl. note that there are two failing test cases, which are supposed to
>> work as per the svn_wc.h documentation. so, I have kept them as-is  
>> (added
>> comments tho). Am trying to figure out the best way to fix these
>> issues/documentation and then would come back and fix these failures.
>>
>> [[[
>> Improve existing tests for the svn.wc python binding.
>>
>> * subversion/bindings/swig/python/tests/wc.py
>>    (global): import shutil.
>>    (test_check_wc): Add invalid-file case.
>>    (test_get_ancestry): Add invalid-file case.
>>    (test_status): Add cases with varying input.
>>    (test_is_normal_prop): Use failUnless instead of assert_.
>>    (test_is_wc_prop): Use failUnless instead of assert_.
>>    (test_is_entry_prop): Use failUnless instead of assert_.
>>    (test_get_pristine_copy_path): Add test for removal of text-base  
>> case.
>> ]]]
> Looks good! Two small comments from a quick glance over your patch:
> Why the assert_ -> failUnless change?

IIUC, assert_ will cause and assertion and hence an error. failUnless will  
cause a failure (test failure)

> I'm not sure whether adding tests that fail are a good idea - it makes
> it harder to catch real regressions. Can you comment out the failing
> ones and a TODO or send a fix along that fixes the actual bug?

Good idea. Will do that, and send the patch again. Thanks. :)

Regards,
Madan.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Improve test cases for svn.wc python bindings

Posted by Jelmer Vernooij <je...@samba.org>.
Hi Madan,

On Sat, 2006-06-17 at 14:54 +0530, Madan U Sreenivasan wrote:
>    Pl. find attached some improved tests for svn.wc python bindings.
> 
>    This patch is dependant on the import of shutil module which is also  
> present in a patch I submitted earlier (but not committed, yet)...
>         http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=116821
I'll try to have a look at it later today (if nobody has committed it
yet by then)

>    Pl. note that there are two failing test cases, which are supposed to  
> work as per the svn_wc.h documentation. so, I have kept them as-is (added  
> comments tho). Am trying to figure out the best way to fix these  
> issues/documentation and then would come back and fix these failures.
> 
> [[[
> Improve existing tests for the svn.wc python binding.
> 
> * subversion/bindings/swig/python/tests/wc.py
>    (global): import shutil.
>    (test_check_wc): Add invalid-file case.
>    (test_get_ancestry): Add invalid-file case.
>    (test_status): Add cases with varying input.
>    (test_is_normal_prop): Use failUnless instead of assert_.
>    (test_is_wc_prop): Use failUnless instead of assert_.
>    (test_is_entry_prop): Use failUnless instead of assert_.
>    (test_get_pristine_copy_path): Add test for removal of text-base case.
> ]]]
Looks good! Two small comments from a quick glance over your patch:
Why the assert_ -> failUnless change? 

I'm not sure whether adding tests that fail are a good idea - it makes
it harder to catch real regressions. Can you comment out the failing
ones and a TODO or send a fix along that fixes the actual bug?

Cheers,

Jelmer
-- 
Jelmer Vernooij <je...@samba.org> - http://samba.org/~jelmer/