You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Garrett Rooney <ro...@electricjellyfish.net> on 2003/02/09 15:27:07 UTC
Re: svn commit: rev 4805 - in trunk: . subversion/tests/libsvn_delta
On Sunday, February 9, 2003, at 08:41 AM, philip@tigris.org wrote:
> Author: philip
> Date: 2003-02-09 07:40:50 -0600 (Sun, 09 Feb 2003)
> New Revision: 4805
>
> Added:
> trunk/subversion/tests/libsvn_delta/diff-diff3-test.c
> Modified:
> trunk/build.conf
> Log:
> If we are to enable the internal diff library then we ought to have
> some regression tests. So this tests some basic behaviour, but does
> not test the conflict handling. Some of these tests fail.
first off, thank you thank you thank you for putting these together, we
absolutely need them.
second, how did you come up with the results for the
two_way_unified_suspect tests? are you just comparing against the
output from gnu diff? at least the first 'failure' in there seems to
produce a valid unified diff, it's just showing slightly more context
than you're expecting.
perhaps these tests shouldn't be comparing against a cannonical 'good
result', but instead should be using patch to try to reconstruct the
original file from the diff, and verifying that it matches.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4805 - in trunk: . subversion/tests/libsvn_delta
Posted by William Uther <wi...@cse.unsw.edu.au>.
On Tuesday, February 11, 2003, at 03:04 PM, Greg Stein wrote:
> On Sun, Feb 09, 2003 at 03:45:11PM +0000, Philip Martin wrote:
>> ...
>> I disagree. A "diff" that simply deleted one file and added the
>> second would do that, but it's not very useful. These tests are
>> designed to prohibit that. Obviously in some cases the diff algorithm
>> has to make arbitrary choices, but in the the cases I have tested
>> explicitly I believe the diff output should be predictable. Any
>> variation it is most likely a bug.
>
> Right.
>
> Is it possible to set up a test whereby we have three subdirectories,
> and we
> just start dropping stuff in there?
>
> 1) source
> 2) destination
> 3) expected diff
>
> A test script can then just take triplets at will and run the test. We
> can
> then just start loading the directories, rather than tweaking C code.
That looks good, but that will only be able to test cases where the
diff is unambiguous. Could you make another three directories for
ambiguous diffs:
1) source
2) destination
3) a file containing the maximum diff size.
Again, the script can grab triplets at will. It can run the diff/patch
test; taking the diff between source and dest, re-applying that diff to
source and then comparing with dest. It can also check that the diff
is below the size specified in the third file. This means:
- We're not checking for identity with the gnu diff algorithm (and
hence limited in terms of implementation), but
- It doesn't allow degenerate diffs like "remove all", "add all"
The maximum diff sizes could be found by running GNU diff over source
and dest, and then adding 5% slack. If you wanted to get really fancy,
the test could even output the average difference in diff size between
the internal diff and gnu diff.
later,
Will :-}
--
Dr William Uther National ICT Australia
Phone: +61 2 9385 6926 School of Computer Science and
Engineering
Email: willu@cse.unsw.edu.au University of New South Wales
Jabber: willu@jabber.cse.unsw.edu.au Sydney, Australia
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 4805 - in trunk: . subversion/tests/libsvn_delta
Posted by Greg Stein <gs...@lyra.org>.
On Sun, Feb 09, 2003 at 03:45:11PM +0000, Philip Martin wrote:
>...
> I disagree. A "diff" that simply deleted one file and added the
> second would do that, but it's not very useful. These tests are
> designed to prohibit that. Obviously in some cases the diff algorithm
> has to make arbitrary choices, but in the the cases I have tested
> explicitly I believe the diff output should be predictable. Any
> variation it is most likely a bug.
Right.
Is it possible to set up a test whereby we have three subdirectories, and we
just start dropping stuff in there?
1) source
2) destination
3) expected diff
A test script can then just take triplets at will and run the test. We can
then just start loading the directories, rather than tweaking C code.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] possible fix for diff coredumps (was Re: svn commit: rev 4805 - in trunk: . subversion/tests/libsvn_delta)
Posted by Sander Striker <st...@apache.org>.
> From: Garrett Rooney [mailto:rooneg@electricjellyfish.net]
> Sent: Sunday, February 09, 2003 6:23 PM
> as for the other parts of the tests (the ones that dump core), here's a
> patch that 'fixes' it, although i'm nowhere near convinced it is
> correct, as i'm not sure how the diff/diff3 code works, so sander will
> have to look over them first.
I did look over it and it is a partial fix. The variables are accessed
later on in the code when there is a conflict to be examined.
Will take a closer look on wednesday.
Thanks,
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] possible fix for diff coredumps (was Re: svn commit: rev 4805 - in trunk: . subversion/tests/libsvn_delta)
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Sunday, February 9, 2003, at 10:45 AM, Philip Martin wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
>
>> at least the first 'failure' in there seems to
>> produce a valid unified diff, it's just showing slightly more context
>> than you're expecting.
>
> Which probably indicates a bug :) What happens when we extend the
> library to support a varying number of context lines? I believe the
> current output is wrong.
>
> Lets see
>
> $ patch --dry-run foo15a < diff-foo15a-bar15a
> patching file foo15a
> Hunk #1 succeeded at 1 with fuzz 1.
>
> See that fuzz message? Even patch thinks it's suspect.
>
>> perhaps these tests shouldn't be comparing against a cannonical 'good
>> result', but instead should be using patch to try to reconstruct the
>> original file from the diff, and verifying that it matches.
>
> I disagree. A "diff" that simply deleted one file and added the
> second would do that, but it's not very useful. These tests are
> designed to prohibit that. Obviously in some cases the diff algorithm
> has to make arbitrary choices, but in the the cases I have tested
> explicitly I believe the diff output should be predictable. Any
> variation it is most likely a bug.
ok, you convinced me ;-)
as for the other parts of the tests (the ones that dump core), here's a
patch that 'fixes' it, although i'm nowhere near convinced it is
correct, as i'm not sure how the diff/diff3 code works, so sander will
have to look over them first.
-garrett
Re: svn commit: rev 4805 - in trunk: . subversion/tests/libsvn_delta
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> at least the first 'failure' in there seems to
> produce a valid unified diff, it's just showing slightly more context
> than you're expecting.
Which probably indicates a bug :) What happens when we extend the
library to support a varying number of context lines? I believe the
current output is wrong.
Lets see
$ patch --dry-run foo15a < diff-foo15a-bar15a
patching file foo15a
Hunk #1 succeeded at 1 with fuzz 1.
See that fuzz message? Even patch thinks it's suspect.
> perhaps these tests shouldn't be comparing against a cannonical 'good
> result', but instead should be using patch to try to reconstruct the
> original file from the diff, and verifying that it matches.
I disagree. A "diff" that simply deleted one file and added the
second would do that, but it's not very useful. These tests are
designed to prohibit that. Obviously in some cases the diff algorithm
has to make arbitrary choices, but in the the cases I have tested
explicitly I believe the diff output should be predictable. Any
variation it is most likely a bug.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
diff tests, WAS: RE: svn commit: rev 4805 - in trunk: . subversion/tests/libsvn_delta
Posted by Sander Striker <st...@apache.org>.
> From: Garrett Rooney [mailto:rooneg@electricjellyfish.net]
> Sent: Sunday, February 09, 2003 4:27 PM
> On Sunday, February 9, 2003, at 08:41 AM, philip@tigris.org wrote:
>> If we are to enable the internal diff library then we ought to have
>> some regression tests. So this tests some basic behaviour, but does
>> not test the conflict handling. Some of these tests fail.
>
> first off, thank you thank you thank you for putting these together, we
> absolutely need them.
Couldn't agree more. Phillip: THANKS!
> second, how did you come up with the results for the
> two_way_unified_suspect tests? are you just comparing against the
> output from gnu diff? at least the first 'failure' in there seems to
> produce a valid unified diff, it's just showing slightly more context
> than you're expecting.
Nah, this is a bug. Fortunately not in the core logic ;). Prolly
has something to do with flushing the current hunk in diff_file.c.
I won't really have time to look at this until wednesday.
> perhaps these tests shouldn't be comparing against a cannonical 'good
> result', but instead should be using patch to try to reconstruct the
> original file from the diff, and verifying that it matches.
That would be a good test if the diffs are unpredictable and multiple
outputs are correct. In this case I believe Phillip crafted the files
so that there would be only one possible output.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org