You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2009/04/04 15:23:24 UTC

Re: svn commit: r36978 - trunk/subversion/tests/cmdline

Okay... you posed the question. What *is* it a revert of, then?

You've made the test conform, but provided no insight into what is
happening or what *should* happen. Is this test alteration simply
tracking the output, or have you settled on a definition and the
output is *supposed* to be that way.

IOW, what have you decided this sequence is supposed to do, and how
does this test ensure that *that* result happens. Add some comments?

In the issue, you say "one of the steps is completely bogus" ... which
one, and bogus how?

I just worry that we haven't actually *fixed* anything. That it
"happens to work" merely because we now expect "some kind of output",
but that there is a missing understanding of the innards.

Thx,
-g

On Fri, Apr 3, 2009 at 20:06, Hyrum K. Wright <hy...@hyrumwright.org> wrote:
> Author: hwright
> Date: Fri Apr  3 11:06:57 2009
> New Revision: 36978
>
> Log:
> Further change the expected output for revert tests 17 for wc-ng.
>
> The original test was written as a response to issue 2804, but it turns out
> that the series of events in the test creates ambiguities.  Is the revert a
> revert of the merge, or a revert of the copy, or the parent or what?  This
> basically represents a deviation from known reality, so we define it as Unreal.
>
> Note that the issue solution is still valid, it's the intermediate steps
> getting there which require modification.
>
> * subversion/tests/cmdline/revert_tests.py
>  (status_of_missing_dir_after_revert_replace_with_history_dir): Change an
>    expectation with wc-ng.
>
> Modified:
>   trunk/subversion/tests/cmdline/revert_tests.py
>
> Modified: trunk/subversion/tests/cmdline/revert_tests.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/revert_tests.py?pathrev=36978&r1=36977&r2=36978
> ==============================================================================
> --- trunk/subversion/tests/cmdline/revert_tests.py      Fri Apr  3 10:29:54 2009        (r36977)
> +++ trunk/subversion/tests/cmdline/revert_tests.py      Fri Apr  3 11:06:57 2009        (r36978)
> @@ -825,10 +825,18 @@ def status_of_missing_dir_after_revert_r
>   svntest.actions.run_and_verify_svn(None, expected_output, [], "revert", "-R",
>                                      G_path)
>
> -  expected_output = svntest.verify.UnorderedOutput(
> -    ["?       " + os.path.join(G_path, "pi") + "\n",
> -     "?       " + os.path.join(G_path, "rho") + "\n",
> -     "?       " + os.path.join(G_path, "tau") + "\n"])
> +  if not sbox.using_wc_ng():
> +    expected_output = svntest.verify.UnorderedOutput(
> +      ["?       " + os.path.join(G_path, "pi") + "\n",
> +       "?       " + os.path.join(G_path, "rho") + "\n",
> +       "?       " + os.path.join(G_path, "tau") + "\n"])
> +  else:
> +    expected_output = svntest.verify.UnorderedOutput(
> +      ["A       " + os.path.join(G_path, "pi") + "\n",
> +       "A       " + os.path.join(G_path, "rho") + "\n",
> +       "A       " + os.path.join(G_path, "alpha") + "\n",
> +       "A       " + os.path.join(G_path, "beta") + "\n",
> +       "A       " + os.path.join(G_path, "tau") + "\n"])
>   svntest.actions.run_and_verify_svn(None, expected_output, [],
>                                      "status", wc_dir)
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1534387
>

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1543554


Re: svn commit: r36978 - trunk/subversion/tests/cmdline

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Apr 4, 2009, at 10:23 AM, Greg Stein wrote:

> Okay... you posed the question. What *is* it a revert of, then?
>
> You've made the test conform, but provided no insight into what is
> happening or what *should* happen. Is this test alteration simply
> tracking the output, or have you settled on a definition and the
> output is *supposed* to be that way.
>
> IOW, what have you decided this sequence is supposed to do, and how
> does this test ensure that *that* result happens. Add some comments?
>
> In the issue, you say "one of the steps is completely bogus" ... which
> one, and bogus how?
>
> I just worry that we haven't actually *fixed* anything. That it
> "happens to work" merely because we now expect "some kind of output",
> but that there is a missing understanding of the innards.

The point of issue 2804, and hence the test was to ensure that a  
directory, if deleted and reverted and then non-svn removed, would  
report '!'.  In other words, the directory would still be versioned,  
even if we don't want it to be.

r36978 does not change the end result of the test, just the  
intermediate results.  wc-ng has changed a bit of the way that we  
handle copyfrom information, which was the difference in this test.   
You and I both agree that the test was probably originally written  
using the "expect whatever the current output is" approach, and that's  
all that I did in r36978.

-Hyrum

>
> On Fri, Apr 3, 2009 at 20:06, Hyrum K. Wright  
> <hy...@hyrumwright.org> wrote:
>> Author: hwright
>> Date: Fri Apr  3 11:06:57 2009
>> New Revision: 36978
>>
>> Log:
>> Further change the expected output for revert tests 17 for wc-ng.
>>
>> The original test was written as a response to issue 2804, but it  
>> turns out
>> that the series of events in the test creates ambiguities.  Is the  
>> revert a
>> revert of the merge, or a revert of the copy, or the parent or  
>> what?  This
>> basically represents a deviation from known reality, so we define  
>> it as Unreal.
>>
>> Note that the issue solution is still valid, it's the intermediate  
>> steps
>> getting there which require modification.
>>
>> * subversion/tests/cmdline/revert_tests.py
>>  (status_of_missing_dir_after_revert_replace_with_history_dir):  
>> Change an
>>    expectation with wc-ng.
>>
>> Modified:
>>   trunk/subversion/tests/cmdline/revert_tests.py
>>
>> Modified: trunk/subversion/tests/cmdline/revert_tests.py
>> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/revert_tests.py?pathrev=36978&r1=36977&r2=36978
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- trunk/subversion/tests/cmdline/revert_tests.py      Fri Apr  3  
>> 10:29:54 2009        (r36977)
>> +++ trunk/subversion/tests/cmdline/revert_tests.py      Fri Apr  3  
>> 11:06:57 2009        (r36978)
>> @@ -825,10 +825,18 @@ def status_of_missing_dir_after_revert_r
>>   svntest.actions.run_and_verify_svn(None, expected_output, [],  
>> "revert", "-R",
>>                                      G_path)
>>
>> -  expected_output = svntest.verify.UnorderedOutput(
>> -    ["?       " + os.path.join(G_path, "pi") + "\n",
>> -     "?       " + os.path.join(G_path, "rho") + "\n",
>> -     "?       " + os.path.join(G_path, "tau") + "\n"])
>> +  if not sbox.using_wc_ng():
>> +    expected_output = svntest.verify.UnorderedOutput(
>> +      ["?       " + os.path.join(G_path, "pi") + "\n",
>> +       "?       " + os.path.join(G_path, "rho") + "\n",
>> +       "?       " + os.path.join(G_path, "tau") + "\n"])
>> +  else:
>> +    expected_output = svntest.verify.UnorderedOutput(
>> +      ["A       " + os.path.join(G_path, "pi") + "\n",
>> +       "A       " + os.path.join(G_path, "rho") + "\n",
>> +       "A       " + os.path.join(G_path, "alpha") + "\n",
>> +       "A       " + os.path.join(G_path, "beta") + "\n",
>> +       "A       " + os.path.join(G_path, "tau") + "\n"])
>>   svntest.actions.run_and_verify_svn(None, expected_output, [],
>>                                      "status", wc_dir)
>>
>> ------------------------------------------------------
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1534387
>>
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1543554

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1563757