[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] commit_tests

From: Stefan Sperling <stsp_at_elego.de>
Date: Sat, 13 Jun 2009 14:22:01 +0100

On Sat, Jun 13, 2009 at 01:54:25PM +0800, HuiHuang wrote:
> Hey Stefan,
> >So instead of adding a new test, we can change the commit_multiple_wc
> >test to expect the commit to succeed instead of fail, and mark the
> >test XFail. Can you make a patch for that?
> Do you mean that I make a patch like this?
> Index: commit_tests.py
> ===================================================================
> --- commit_tests.py (版本 38022)
> +++ commit_tests.py (工作副本)
> @@ -1393,6 +1393,7 @@
> # Verify status unchanged
> svntest.actions.run_and_verify_status(wc_dir, expected_status)
> svntest.actions.run_and_verify_status(wc2_dir, expected_status2)
> + raise svntest.Failure

Raising Failure like that is almost never the right thing to do!

In tests, we usually want the failure or success to come from
an invocation of svntest.actions.run_and_verify_status().
Otherwise we are not testing Subversion. We are just writing
code which raises exceptions but doesn't test anything.

So, to be clear, I meant something like:

Index: subversion/tests/cmdline/commit_tests.py
--- subversion/tests/cmdline/commit_tests.py (revision 38018)
+++ subversion/tests/cmdline/commit_tests.py (working copy)
@@ -1353,8 +1353,11 @@
 # Commit from multiple working copies is not yet supported. At
 # present an error is generated and none of the working copies change.
-# Related to issue 959, this test here doesn't use svn:externals but the
+# Related to issue #959, this test here doesn't use svn:externals but the
 # behaviour needs to be considered.
+# Also related to issue #2381, "Cannot commit multiple WC paths which
+# lack a common parent directory". Once that issue has been resolved,
+# we will remove the XFail from this test.
 def commit_multiple_wc(sbox):
   "attempted commit from multiple wc fails"
@@ -1384,14 +1387,16 @@
   expected_status2.tweak('A/B/lambda', status='M ')
   svntest.actions.run_and_verify_status(wc2_dir, expected_status2)
- # Commit should fail, even though one target is a "child" of the other.
- svntest.actions.run_and_verify_svn("Unexpectedly not locked",
- None, svntest.verify.AnyOutput,
+ # Commit should succeed, even though one target is a "child" of the other,
+ # since both working copies come from the same repository.
+ svntest.actions.run_and_verify_svn(None, None, svntest.verify.AnyOutput,
                                      'commit', '-m', 'log',
                                      wc_dir, wc2_dir)
- # Verify status unchanged
+ # Verify status changed
+ expected_status.tweak('A/mu', status=' ', wc_rev=2)
   svntest.actions.run_and_verify_status(wc_dir, expected_status)
+ expected_status2.tweak('A/B/lambda', status=' ', wc_rev=2)
   svntest.actions.run_and_verify_status(wc2_dir, expected_status2)
@@ -2696,7 +2701,7 @@
- commit_multiple_wc,
+ XFail(commit_multiple_wc),

Do you see the difference between doing the above and what you proposed?

Received on 2009-06-13 15:22:36 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.