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 2010/08/03 17:44:02 UTC
Re: svn commit: r981885 - /subversion/trunk/subversion/tests/cmdline/resolved_tests.py
These are *regression* tests. You can't just make them pass by changing them.
Let's see some rationale!! Why should the output change? And if it
does, then there better be some commentary about WHY that is. I see no
comments about why this is allowed to change. No explanation. No
nothing.
The tests should produce the exact same result. That is why they are
there. To ensure we haven't buggered something up.
Any time the output is supposed to be different now, then we need a
full explanation on why that has happened. We may need to write an
errata. We may need to update documentation. Or more likely, we need
to fix some bugs.
Why are all these tests changing? This doesn't seem right.
-g
On Tue, Aug 3, 2010 at 09:56, <ph...@apache.org> wrote:
> Author: philip
> Date: Tue Aug 3 13:56:58 2010
> New Revision: 981885
>
> URL: http://svn.apache.org/viewvc?rev=981885&view=rev
> Log:
> Make resolved_tests.py 1 pass in single-db.
>
> * subversion/tests/cmdline/switc_tests.py
> (resolved_on_wc_root): Tweak expectations for single-db.
>
> Modified:
> subversion/trunk/subversion/tests/cmdline/resolved_tests.py
>
> Modified: subversion/trunk/subversion/tests/cmdline/resolved_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/resolved_tests.py?rev=981885&r1=981884&r2=981885&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/resolved_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/resolved_tests.py Tue Aug 3 13:56:58 2010
> @@ -112,6 +112,8 @@ def resolved_on_wc_root(sbox):
> 'A/B/lambda',
> 'A/B/E/alpha', 'A/B/E/beta',
> 'A/D/gamma')
> + if svntest.main.wc_is_singledb(sbox.wc_dir):
> + expected_disk.remove('A/B/E', 'A/B/F', 'A/B')
>
> expected_status = svntest.actions.get_virginal_state(wc, 2)
> expected_status.tweak('iota', 'A/B', 'A/D/gamma',
>
>
>
Re: svn commit: r981885 - /subversion/trunk/subversion/tests/cmdline/resolved_tests.py
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
I understand and agree with Greg that we may want to be a bit more
explanatory in commit comments.
That being said, out test suite has a nasty habit of itself relying
too deeply upon implementation details. Historically, when you
recursively remove a directory, the directories have to stick around
(so you have a .svn in which to record the fact that they've been
deleted), which is strictly an implementation detail. The fact is
that the test suite cares too much about such things, hence the need
to update the expectations in this (and other similar cases).
Oh, and never mind the fact that we conflate a number of different
issues in the test suite. Was the test a performance test, regression
test, acceptance test, or some other kind of test? We don't record
that information, nor do we separate the various types in the test
suite, which makes knowing whether a change like this is valid even
harder.
So, I agree that a bit more explanation is in order, but I partly
blame the test suite that such explanation is even required.
-Hyrum
On Tue, Aug 3, 2010 at 12:44 PM, Greg Stein <gs...@gmail.com> wrote:
> These are *regression* tests. You can't just make them pass by changing them.
>
> Let's see some rationale!! Why should the output change? And if it
> does, then there better be some commentary about WHY that is. I see no
> comments about why this is allowed to change. No explanation. No
> nothing.
>
> The tests should produce the exact same result. That is why they are
> there. To ensure we haven't buggered something up.
>
> Any time the output is supposed to be different now, then we need a
> full explanation on why that has happened. We may need to write an
> errata. We may need to update documentation. Or more likely, we need
> to fix some bugs.
>
> Why are all these tests changing? This doesn't seem right.
>
> -g
>
> On Tue, Aug 3, 2010 at 09:56, <ph...@apache.org> wrote:
>> Author: philip
>> Date: Tue Aug 3 13:56:58 2010
>> New Revision: 981885
>>
>> URL: http://svn.apache.org/viewvc?rev=981885&view=rev
>> Log:
>> Make resolved_tests.py 1 pass in single-db.
>>
>> * subversion/tests/cmdline/switc_tests.py
>> (resolved_on_wc_root): Tweak expectations for single-db.
>>
>> Modified:
>> subversion/trunk/subversion/tests/cmdline/resolved_tests.py
>>
>> Modified: subversion/trunk/subversion/tests/cmdline/resolved_tests.py
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/resolved_tests.py?rev=981885&r1=981884&r2=981885&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/tests/cmdline/resolved_tests.py (original)
>> +++ subversion/trunk/subversion/tests/cmdline/resolved_tests.py Tue Aug 3 13:56:58 2010
>> @@ -112,6 +112,8 @@ def resolved_on_wc_root(sbox):
>> 'A/B/lambda',
>> 'A/B/E/alpha', 'A/B/E/beta',
>> 'A/D/gamma')
>> + if svntest.main.wc_is_singledb(sbox.wc_dir):
>> + expected_disk.remove('A/B/E', 'A/B/F', 'A/B')
>>
>> expected_status = svntest.actions.get_virginal_state(wc, 2)
>> expected_status.tweak('iota', 'A/B', 'A/D/gamma',
>>
>>
>>
>