[PATCH] test: make test_expect_equal_file() arguments flexible

[David Bremner]

Dmitry Kurochkin  writes:

> Before the change, test_expect_equal_file() function treated the first
> argument as "actual output file" and the second argument as "expected
> output file".  When the test fails, the files are copied for later
> inspection.  The first files was copied to "$testname.output" and the
> second file to "$testname.expected".  The argument order for
> test_expect_equal_file() is often wrong which results in confusing
> diff output and incorrectly named files.

pushed, 

d

[PATCH] test: make test_expect_equal_file() arguments flexible

[Tomi Ollila]

On Thu, 02 Feb 2012 14:33:56 +, David Edmondson  wrote:
> On Wed, 01 Feb 2012 09:24:32 -0800, Jameson Graef Rollins  finestructure.net> wrote:
> > If this is really a problem, I vote for 1.  In general, I am not in
> > favor of making the test suite more complicated than it needs to be.
> 
> After listening to the debate, I agree. The documentation should state
> that the order is 'expected actual' (or the other way around) and
> offenders should be shot on sight^W^W^Wfixed.

I've started to agree with Dmitry.

Why do something that computer can do -- to guide test writers to
give args in consistent order and provide suitable filenames.

Secondly as the output files are provided for human consumption
if there is consistent naming in expected output files helps 
developers getting parts of the big picture easier and finding 
right filenames easier.

... and the reviewers doesn't need to keep their plasma guns
handly.

Tomi

[PATCH] test: make test_expect_equal_file() arguments flexible

[David Edmondson]

On Wed, 01 Feb 2012 09:24:32 -0800, Jameson Graef Rollins  wrote:
> If this is really a problem, I vote for 1.  In general, I am not in
> favor of making the test suite more complicated than it needs to be.

After listening to the debate, I agree. The documentation should state
that the order is 'expected actual' (or the other way around) and
offenders should be shot on sight^W^W^Wfixed.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 

[PATCH] test: make test_expect_equal_file() arguments flexible

[Jameson Graef Rollins]

On Wed,  1 Feb 2012 11:19:54 +0400, Dmitry Kurochkin  wrote:
> Before the change, test_expect_equal_file() function treated the first
> argument as "actual output file" and the second argument as "expected
> output file".  When the test fails, the files are copied for later
> inspection.  The first files was copied to "$testname.output" and the
> second file to "$testname.expected".  The argument order for
> test_expect_equal_file() is often wrong which results in confusing
> diff output and incorrectly named files.

After thinking about this some more, I'm taking it all back.  I think
this is a fine solution, since it just goes with whatever name the test
is invoked with.  Since this is usually just OUTPUT and EXPECTED, it
should all be clear, and the diffs should be fine, even if the ordering
is permuted some places.  Sorry about all the chatter.

+1

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 

[PATCH] test: make test_expect_equal_file() arguments flexible

[Dmitry Kurochkin]

On Thu, 02 Feb 2012 03:42:29 +0400, Dmitry Kurochkin  wrote:
> Hi Jameson.
> 
> On Wed, 01 Feb 2012 09:24:32 -0800, Jameson Graef Rollins  finestructure.net> wrote:
> > On Wed, 01 Feb 2012 14:37:53 +0400, Dmitry Kurochkin  > gmail.com> wrote:
> > > On Wed, 01 Feb 2012 12:18:08 +0200, Tomi Ollila  
> > > wrote:
> > > > 
> > > > There are at least these options here
> > > > 
> > > > 1) go through all ~100 places where test_expect_equal_file is used
> > > >and fix the call order: quick look tells that the offending uses
> > > >are in dump-restore, hooks, search-limiting and symbol-hiding.
> > > > 
> > > > 2) enforce "expected" filename has some format *and* fix all current
> > > >uses of it. Add testbed_error () function which yells loudly ane 
> > > > exits...
> > > > 
> > > > 3) guess which is output and which is expected from args so that 
> > > >machine helps tester here (for both diff output & copied files)a
> > > > 
> > > > 4) just copy compared files to some directory, those are named as
> > > >basename of the original -- diff order still inconsistent.
> > > > 
> > > > 
> > > > I'd just go with option 1 and fix new *violations* when stumble upon 
> > > > one.
> > > > 
> > > 
> > > Option 1 does not solve the problem.  New violations would apper and
> > > need to be fixed again.  I am for option 2.
> > 
> > How is enforcing use of a particular filename better and more robust
> > than enforcing argument order?
> 
> Filename check is a way to make sure the argument order is correct.
> 
> >  You'll still have to force an arbitrary
> > heuristic.  And you'll still be vulnerable to people messing up the file
> > names (which actually seems easier to get wrong than messing up the
> > order).
> 
> Do you mean that people would start writing tests with filenames like:
> 
>   test_expect_equal_file EXPECTED1 EXPECTED2
> 
> ?  That is possible, of course.  But do you seriously believe that
> deliberately changing file names in a way that violates common sense and
> is inconsistent with all other code is "easier" than writing "EXPECTED
> OUTPUT" in the wrong order?  I do not think so.  And it would definately
> be easier to catch during review.
> 
> >  And you'll have to have more code to parse the argument
> > strings.
> 
> That is one case statement, with one non-empty case, with one error
> call.
> 
> >  And you'll still get inconsistent diffs.
> > 
> 
> No, you do not.  That is the point.
> 
> > If this is really a problem, I vote for 1.  In general, I am not in
> > favor of making the test suite more complicated than it needs to be.
> > 
> 
> Ok.  I plan to send a patch soon.  If my arguments do not convince
> enough people, I will move along.
> 

The new patches, based on idea suggested by Tomi [1], are here:

  id:"1328141050-30356-1-git-send-email-dmitry.kurochkin at gmail.com"

Regards,
  Dmitry

[1] id:"m28vknaq5l.fsf at guru.guru-group.fi"

> Regards,
>   Dmitry
> 
> > jamie.

[PATCH] test: make test_expect_equal_file() arguments flexible

[Dmitry Kurochkin]

Hi Jameson.

On Wed, 01 Feb 2012 09:24:32 -0800, Jameson Graef Rollins  wrote:
> On Wed, 01 Feb 2012 14:37:53 +0400, Dmitry Kurochkin  gmail.com> wrote:
> > On Wed, 01 Feb 2012 12:18:08 +0200, Tomi Ollila  
> > wrote:
> > > 
> > > There are at least these options here
> > > 
> > > 1) go through all ~100 places where test_expect_equal_file is used
> > >and fix the call order: quick look tells that the offending uses
> > >are in dump-restore, hooks, search-limiting and symbol-hiding.
> > > 
> > > 2) enforce "expected" filename has some format *and* fix all current
> > >uses of it. Add testbed_error () function which yells loudly ane 
> > > exits...
> > > 
> > > 3) guess which is output and which is expected from args so that 
> > >machine helps tester here (for both diff output & copied files)a
> > > 
> > > 4) just copy compared files to some directory, those are named as
> > >basename of the original -- diff order still inconsistent.
> > > 
> > > 
> > > I'd just go with option 1 and fix new *violations* when stumble upon one.
> > > 
> > 
> > Option 1 does not solve the problem.  New violations would apper and
> > need to be fixed again.  I am for option 2.
> 
> How is enforcing use of a particular filename better and more robust
> than enforcing argument order?

Filename check is a way to make sure the argument order is correct.

>  You'll still have to force an arbitrary
> heuristic.  And you'll still be vulnerable to people messing up the file
> names (which actually seems easier to get wrong than messing up the
> order).

Do you mean that people would start writing tests with filenames like:

  test_expect_equal_file EXPECTED1 EXPECTED2

?  That is possible, of course.  But do you seriously believe that
deliberately changing file names in a way that violates common sense and
is inconsistent with all other code is "easier" than writing "EXPECTED
OUTPUT" in the wrong order?  I do not think so.  And it would definately
be easier to catch during review.

>  And you'll have to have more code to parse the argument
> strings.

That is one case statement, with one non-empty case, with one error
call.

>  And you'll still get inconsistent diffs.
> 

No, you do not.  That is the point.

> If this is really a problem, I vote for 1.  In general, I am not in
> favor of making the test suite more complicated than it needs to be.
> 

Ok.  I plan to send a patch soon.  If my arguments do not convince
enough people, I will move along.

Regards,
  Dmitry

> jamie.

Re: [PATCH] test: make test_expect_equal_file() arguments flexible

[Jameson Graef Rollins]

On Wed, 01 Feb 2012 14:37:53 +0400, Dmitry Kurochkin 
dmitry.kuroch...@gmail.com wrote:
 On Wed, 01 Feb 2012 12:18:08 +0200, Tomi Ollila tomi.oll...@iki.fi wrote:
  
  There are at least these options here
  
  1) go through all ~100 places where test_expect_equal_file is used
 and fix the call order: quick look tells that the offending uses
 are in dump-restore, hooks, search-limiting and symbol-hiding.
  
  2) enforce expected filename has some format *and* fix all current
 uses of it. Add testbed_error () function which yells loudly ane exits...
  
  3) guess which is output and which is expected from args so that 
 machine helps tester here (for both diff output  copied files)a
  
  4) just copy compared files to some directory, those are named as
 basename of the original -- diff order still inconsistent.
  
  
  I'd just go with option 1 and fix new *violations* when stumble upon one.
  
 
 Option 1 does not solve the problem.  New violations would apper and
 need to be fixed again.  I am for option 2.

How is enforcing use of a particular filename better and more robust
than enforcing argument order?  You'll still have to force an arbitrary
heuristic.  And you'll still be vulnerable to people messing up the file
names (which actually seems easier to get wrong than messing up the
order).  And you'll have to have more code to parse the argument
strings.  And you'll still get inconsistent diffs.

If this is really a problem, I vote for 1.  In general, I am not in
favor of making the test suite more complicated than it needs to be.

jamie.


pgp6nDcm3NScF.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] test: make test_expect_equal_file() arguments flexible

[Dmitry Kurochkin]

Hi Jameson.

On Wed, 01 Feb 2012 09:24:32 -0800, Jameson Graef Rollins 
jroll...@finestructure.net wrote:
 On Wed, 01 Feb 2012 14:37:53 +0400, Dmitry Kurochkin 
 dmitry.kuroch...@gmail.com wrote:
  On Wed, 01 Feb 2012 12:18:08 +0200, Tomi Ollila tomi.oll...@iki.fi wrote:
   
   There are at least these options here
   
   1) go through all ~100 places where test_expect_equal_file is used
  and fix the call order: quick look tells that the offending uses
  are in dump-restore, hooks, search-limiting and symbol-hiding.
   
   2) enforce expected filename has some format *and* fix all current
  uses of it. Add testbed_error () function which yells loudly ane 
   exits...
   
   3) guess which is output and which is expected from args so that 
  machine helps tester here (for both diff output  copied files)a
   
   4) just copy compared files to some directory, those are named as
  basename of the original -- diff order still inconsistent.
   
   
   I'd just go with option 1 and fix new *violations* when stumble upon one.
   
  
  Option 1 does not solve the problem.  New violations would apper and
  need to be fixed again.  I am for option 2.
 
 How is enforcing use of a particular filename better and more robust
 than enforcing argument order?

Filename check is a way to make sure the argument order is correct.

  You'll still have to force an arbitrary
 heuristic.  And you'll still be vulnerable to people messing up the file
 names (which actually seems easier to get wrong than messing up the
 order).

Do you mean that people would start writing tests with filenames like:

  test_expect_equal_file EXPECTED1 EXPECTED2

?  That is possible, of course.  But do you seriously believe that
deliberately changing file names in a way that violates common sense and
is inconsistent with all other code is easier than writing EXPECTED
OUTPUT in the wrong order?  I do not think so.  And it would definately
be easier to catch during review.

  And you'll have to have more code to parse the argument
 strings.

That is one case statement, with one non-empty case, with one error
call.

  And you'll still get inconsistent diffs.
 

No, you do not.  That is the point.

 If this is really a problem, I vote for 1.  In general, I am not in
 favor of making the test suite more complicated than it needs to be.
 

Ok.  I plan to send a patch soon.  If my arguments do not convince
enough people, I will move along.

Regards,
  Dmitry

 jamie.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] test: make test_expect_equal_file() arguments flexible

[Dmitry Kurochkin]

On Thu, 02 Feb 2012 03:42:29 +0400, Dmitry Kurochkin 
dmitry.kuroch...@gmail.com wrote:
 Hi Jameson.
 
 On Wed, 01 Feb 2012 09:24:32 -0800, Jameson Graef Rollins 
 jroll...@finestructure.net wrote:
  On Wed, 01 Feb 2012 14:37:53 +0400, Dmitry Kurochkin 
  dmitry.kuroch...@gmail.com wrote:
   On Wed, 01 Feb 2012 12:18:08 +0200, Tomi Ollila tomi.oll...@iki.fi 
   wrote:

There are at least these options here

1) go through all ~100 places where test_expect_equal_file is used
   and fix the call order: quick look tells that the offending uses
   are in dump-restore, hooks, search-limiting and symbol-hiding.

2) enforce expected filename has some format *and* fix all current
   uses of it. Add testbed_error () function which yells loudly ane 
exits...

3) guess which is output and which is expected from args so that 
   machine helps tester here (for both diff output  copied files)a

4) just copy compared files to some directory, those are named as
   basename of the original -- diff order still inconsistent.


I'd just go with option 1 and fix new *violations* when stumble upon 
one.

   
   Option 1 does not solve the problem.  New violations would apper and
   need to be fixed again.  I am for option 2.
  
  How is enforcing use of a particular filename better and more robust
  than enforcing argument order?
 
 Filename check is a way to make sure the argument order is correct.
 
   You'll still have to force an arbitrary
  heuristic.  And you'll still be vulnerable to people messing up the file
  names (which actually seems easier to get wrong than messing up the
  order).
 
 Do you mean that people would start writing tests with filenames like:
 
   test_expect_equal_file EXPECTED1 EXPECTED2
 
 ?  That is possible, of course.  But do you seriously believe that
 deliberately changing file names in a way that violates common sense and
 is inconsistent with all other code is easier than writing EXPECTED
 OUTPUT in the wrong order?  I do not think so.  And it would definately
 be easier to catch during review.
 
   And you'll have to have more code to parse the argument
  strings.
 
 That is one case statement, with one non-empty case, with one error
 call.
 
   And you'll still get inconsistent diffs.
  
 
 No, you do not.  That is the point.
 
  If this is really a problem, I vote for 1.  In general, I am not in
  favor of making the test suite more complicated than it needs to be.
  
 
 Ok.  I plan to send a patch soon.  If my arguments do not convince
 enough people, I will move along.
 

The new patches, based on idea suggested by Tomi [1], are here:

  id:1328141050-30356-1-git-send-email-dmitry.kuroch...@gmail.com

Regards,
  Dmitry

[1] id:m28vknaq5l@guru.guru-group.fi

 Regards,
   Dmitry
 
  jamie.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] test: make test_expect_equal_file() arguments flexible

[David Edmondson]

On Wed, 01 Feb 2012 09:24:32 -0800, Jameson Graef Rollins 
jroll...@finestructure.net wrote:
 If this is really a problem, I vote for 1.  In general, I am not in
 favor of making the test suite more complicated than it needs to be.

After listening to the debate, I agree. The documentation should state
that the order is 'expected actual' (or the other way around) and
offenders should be shot on sight^W^W^Wfixed.


pgpmJsBZfL7cu.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] test: make test_expect_equal_file() arguments flexible

[Tomi Ollila]

On Thu, 02 Feb 2012 14:33:56 +, David Edmondson d...@dme.org wrote:
 On Wed, 01 Feb 2012 09:24:32 -0800, Jameson Graef Rollins 
 jroll...@finestructure.net wrote:
  If this is really a problem, I vote for 1.  In general, I am not in
  favor of making the test suite more complicated than it needs to be.
 
 After listening to the debate, I agree. The documentation should state
 that the order is 'expected actual' (or the other way around) and
 offenders should be shot on sight^W^W^Wfixed.

I've started to agree with Dmitry.

Why do something that computer can do -- to guide test writers to
give args in consistent order and provide suitable filenames.

Secondly as the output files are provided for human consumption
if there is consistent naming in expected output files helps 
developers getting parts of the big picture easier and finding 
right filenames easier.

... and the reviewers doesn't need to keep their plasma guns
handly.

Tomi

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] test: make test_expect_equal_file() arguments flexible

[Dmitry Kurochkin]

On Wed, 01 Feb 2012 12:18:08 +0200, Tomi Ollila  wrote:
> 
> There are at least these options here
> 
> 1) go through all ~100 places where test_expect_equal_file is used
>and fix the call order: quick look tells that the offending uses
>are in dump-restore, hooks, search-limiting and symbol-hiding.
> 
> 2) enforce "expected" filename has some format *and* fix all current
>uses of it. Add testbed_error () function which yells loudly ane exits...
> 
> 3) guess which is output and which is expected from args so that 
>machine helps tester here (for both diff output & copied files)a
> 
> 4) just copy compared files to some directory, those are named as
>basename of the original -- diff order still inconsistent.
> 
> 
> I'd just go with option 1 and fix new *violations* when stumble upon one.
> 

Option 1 does not solve the problem.  New violations would apper and
need to be fixed again.  I am for option 2.

Regards,
  Dmitry

> Tomi

[PATCH] test: make test_expect_equal_file() arguments flexible

[Dmitry Kurochkin]

On Wed, 01 Feb 2012 10:55:50 +0200, Tomi Ollila  wrote:
> On Wed, 01 Feb 2012 00:47:30 -0800, Jameson Graef Rollins  finestructure.net> wrote:
> > On Wed,  1 Feb 2012 11:19:54 +0400, Dmitry Kurochkin  > gmail.com> wrote:
> > > The down side of this approach is that diff argument order depends on
> > > test_expect_equal_file() argument order.  So sometimes we get diff
> > > from expected to actual results, and sometimes the other way around.
> > > But the files are always named correctly.
> > 
> > Actually, I think this last point is the most important thing to retain.
> > Consistency in the diffs makes reading test results much more efficient.
> > The order I don't much care about.  But seeing as we have been
> > consistent with a particular order for a while, it seems like more
> > effort than it's worth to change it.
> 
> how about enforcing it in the test suite ?
> 
> case $file2 in 
>   *.expected|*.EXPECTED) ;; 
> *) echo "$file2" does not end with 'expected' >&2; exit 1
> esac
> 

It would break some existing tests.  But I like it.

Regards,
  Dmitry

> > 
> > jamie.
> 
> Tomi

[PATCH] test: make test_expect_equal_file() arguments flexible

[Dmitry Kurochkin]

Before the change, test_expect_equal_file() function treated the first
argument as "actual output file" and the second argument as "expected
output file".  When the test fails, the files are copied for later
inspection.  The first files was copied to "$testname.output" and the
second file to "$testname.expected".  The argument order for
test_expect_equal_file() is often wrong which results in confusing
diff output and incorrectly named files.

The patch solves the issue by changing test_expect_equal_file() to
treat arguments just as two files, without any special properties
(like "actual" and "expected").  The file names for copying is now
based on the given file name: "$testname.$file1" and
"$testname.$file2".  E.g. if test_expect_equal_file() is called with
"OUTPUT" and "EXPECTED", the copied files can be named
"emacs.1.OUTPUT" and "emacs.1.EXPECTED".

The down side of this approach is that diff argument order depends on
test_expect_equal_file() argument order.  So sometimes we get diff
from expected to actual results, and sometimes the other way around.
But the files are always named correctly.
---
 test/README  |   10 +-
 test/test-lib.sh |   12 ++--
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/test/README b/test/README
index 43656a3..e0364e8 100644
--- a/test/README
+++ b/test/README
@@ -176,12 +176,12 @@ library for your script to use.
will generate a failure and print the difference of the two
strings.

- test_expect_equal_file  
+ test_expect_equal_file  

-   Identical to test_exepect_equal, except that  and
-are files instead of strings.  This is a much more
-   robust method to compare formatted textual information, since it
-   also notices whitespace and closing newline differences.
+   Identical to test_exepect_equal, except that  and 
+   are files instead of strings.  This is a much more robust method to
+   compare formatted textual information, since it also notices
+   whitespace and closing newline differences.

  test_debug 

[PATCH] test: make test_expect_equal_file() arguments flexible

[Tomi Ollila]

On Wed, 01 Feb 2012 00:47:30 -0800, Jameson Graef Rollins  wrote:
> On Wed,  1 Feb 2012 11:19:54 +0400, Dmitry Kurochkin  gmail.com> wrote:
> > The down side of this approach is that diff argument order depends on
> > test_expect_equal_file() argument order.  So sometimes we get diff
> > from expected to actual results, and sometimes the other way around.
> > But the files are always named correctly.
> 
> Actually, I think this last point is the most important thing to retain.
> Consistency in the diffs makes reading test results much more efficient.
> The order I don't much care about.  But seeing as we have been
> consistent with a particular order for a while, it seems like more
> effort than it's worth to change it.

how about enforcing it in the test suite ?

case $file2 in 
*.expected|*.EXPECTED) ;; 
*) echo "$file2" does not end with 'expected' >&2; exit 1
esac

> 
> jamie.

Tomi

[PATCH] test: make test_expect_equal_file() arguments flexible

[Jameson Graef Rollins]

On Wed, 01 Feb 2012 14:37:53 +0400, Dmitry Kurochkin  wrote:
> On Wed, 01 Feb 2012 12:18:08 +0200, Tomi Ollila  wrote:
> > 
> > There are at least these options here
> > 
> > 1) go through all ~100 places where test_expect_equal_file is used
> >and fix the call order: quick look tells that the offending uses
> >are in dump-restore, hooks, search-limiting and symbol-hiding.
> > 
> > 2) enforce "expected" filename has some format *and* fix all current
> >uses of it. Add testbed_error () function which yells loudly ane exits...
> > 
> > 3) guess which is output and which is expected from args so that 
> >machine helps tester here (for both diff output & copied files)a
> > 
> > 4) just copy compared files to some directory, those are named as
> >basename of the original -- diff order still inconsistent.
> > 
> > 
> > I'd just go with option 1 and fix new *violations* when stumble upon one.
> > 
> 
> Option 1 does not solve the problem.  New violations would apper and
> need to be fixed again.  I am for option 2.

How is enforcing use of a particular filename better and more robust
than enforcing argument order?  You'll still have to force an arbitrary
heuristic.  And you'll still be vulnerable to people messing up the file
names (which actually seems easier to get wrong than messing up the
order).  And you'll have to have more code to parse the argument
strings.  And you'll still get inconsistent diffs.

If this is really a problem, I vote for 1.  In general, I am not in
favor of making the test suite more complicated than it needs to be.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 

[PATCH] test: make test_expect_equal_file() arguments flexible

[David Edmondson]

+1.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 

[PATCH] test: make test_expect_equal_file() arguments flexible

[Jameson Graef Rollins]

On Wed,  1 Feb 2012 11:19:54 +0400, Dmitry Kurochkin  wrote:
> The down side of this approach is that diff argument order depends on
> test_expect_equal_file() argument order.  So sometimes we get diff
> from expected to actual results, and sometimes the other way around.
> But the files are always named correctly.

Actually, I think this last point is the most important thing to retain.
Consistency in the diffs makes reading test results much more efficient.
The order I don't much care about.  But seeing as we have been
consistent with a particular order for a while, it seems like more
effort than it's worth to change it.

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 

Re: [PATCH] test: make test_expect_equal_file() arguments flexible

[David Edmondson]

+1.


pgpjegpgs82Ip.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] test: make test_expect_equal_file() arguments flexible

[Jameson Graef Rollins]

On Wed,  1 Feb 2012 11:19:54 +0400, Dmitry Kurochkin 
dmitry.kuroch...@gmail.com wrote:
 The down side of this approach is that diff argument order depends on
 test_expect_equal_file() argument order.  So sometimes we get diff
 from expected to actual results, and sometimes the other way around.
 But the files are always named correctly.

Actually, I think this last point is the most important thing to retain.
Consistency in the diffs makes reading test results much more efficient.
The order I don't much care about.  But seeing as we have been
consistent with a particular order for a while, it seems like more
effort than it's worth to change it.

jamie.


pgpwdnq3evnhT.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] test: make test_expect_equal_file() arguments flexible

[Tomi Ollila]

On Wed, 01 Feb 2012 00:47:30 -0800, Jameson Graef Rollins 
jroll...@finestructure.net wrote:
 On Wed,  1 Feb 2012 11:19:54 +0400, Dmitry Kurochkin 
 dmitry.kuroch...@gmail.com wrote:
  The down side of this approach is that diff argument order depends on
  test_expect_equal_file() argument order.  So sometimes we get diff
  from expected to actual results, and sometimes the other way around.
  But the files are always named correctly.
 
 Actually, I think this last point is the most important thing to retain.
 Consistency in the diffs makes reading test results much more efficient.
 The order I don't much care about.  But seeing as we have been
 consistent with a particular order for a while, it seems like more
 effort than it's worth to change it.

how about enforcing it in the test suite ?

case $file2 in 
*.expected|*.EXPECTED) ;; 
*) echo $file2 does not end with 'expected' 2; exit 1
esac

 
 jamie.

Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] test: make test_expect_equal_file() arguments flexible

[Dmitry Kurochkin]

Before the change, test_expect_equal_file() function treated the first
argument as actual output file and the second argument as expected
output file.  When the test fails, the files are copied for later
inspection.  The first files was copied to $testname.output and the
second file to $testname.expected.  The argument order for
test_expect_equal_file() is often wrong which results in confusing
diff output and incorrectly named files.

The patch solves the issue by changing test_expect_equal_file() to
treat arguments just as two files, without any special properties
(like actual and expected).  The file names for copying is now
based on the given file name: $testname.$file1 and
$testname.$file2.  E.g. if test_expect_equal_file() is called with
OUTPUT and EXPECTED, the copied files can be named
emacs.1.OUTPUT and emacs.1.EXPECTED.

The down side of this approach is that diff argument order depends on
test_expect_equal_file() argument order.  So sometimes we get diff
from expected to actual results, and sometimes the other way around.
But the files are always named correctly.
---
 test/README  |   10 +-
 test/test-lib.sh |   12 ++--
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/test/README b/test/README
index 43656a3..e0364e8 100644
--- a/test/README
+++ b/test/README
@@ -176,12 +176,12 @@ library for your script to use.
will generate a failure and print the difference of the two
strings.
 
- test_expect_equal_file output expected
+ test_expect_equal_file file1 file2
 
-   Identical to test_exepect_equal, except that output and
-   expected are files instead of strings.  This is a much more
-   robust method to compare formatted textual information, since it
-   also notices whitespace and closing newline differences.
+   Identical to test_exepect_equal, except that file1 and file2
+   are files instead of strings.  This is a much more robust method to
+   compare formatted textual information, since it also notices
+   whitespace and closing newline differences.
 
  test_debug script
 
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 8158328..581b8be 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -488,17 +488,17 @@ test_expect_equal_file ()
test $# = 2 ||
error bug in the test script: not 2 or 3 parameters to 
test_expect_equal
 
-   output=$1
-   expected=$2
+   file1=$1
+   file2=$2
if ! test_skip $test_subtest_name
then
-   if diff -q $expected $output /dev/null ; then
+   if diff -q $file1 $file2 /dev/null ; then
test_ok_ $test_subtest_name
else
testname=$this_test.$test_count
-   cp $output $testname.output
-   cp $expected $testname.expected
-   test_failure_ $test_subtest_name $(diff -u 
$testname.expected $testname.output)
+   cp $file1 $testname.$file1
+   cp $file2 $testname.$file2
+   test_failure_ $test_subtest_name $(diff -u 
$testname.$file1 $testname.$file2)
fi
 fi
 }
-- 
1.7.9

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch