unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
From: Tomi Ollila <tomi.ollila@iki.fi>
To: David Bremner <david@tethera.net>,
	David Bremner <david@tethera.net>,
	notmuch@notmuchmail.org
Subject: Re: [PATCH] test: swap order of arguments of test_expect_equal
Date: Thu, 06 Apr 2017 19:24:45 +0300	[thread overview]
Message-ID: <m2pogpwlki.fsf@guru.guru-group.fi> (raw)
In-Reply-To: <20170406011250.25153-1-david@tethera.net>

On Thu, Apr 06 2017, David Bremner <david@tethera.net> wrote:

> For some reason (probably inherited from git), the order arguments for
> test_expect_equal was "$output $expected"; this again matters when
> generating diffs.
> ---

IMO these both of these should use expected - output argument order.

But more important than that is to get 
id:20170405003630.15104-1-david@tethera.net merged soon so confusion
of output ends and if someone(tm) is doing near-future test changes
risk of collisions can be avoided.

Tomi


>
> I'm less sure about this one so I didn't go through and update all of
> the tests.  The existing use of test_expect_equal is mostly "$output"
> "$expected", which is natural enough, except that it's the opposite of
> the convention for test_expect_equal_file that I thought was obvious
> (I guess the latter motivated by the arguments to diff).
>
>  test/T000-basic.sh                         | 4 ++--
>  test/test-lib.sh                           | 5 +++--
>  test/test.expected-output/test-verbose-no  | 4 ++--
>  test/test.expected-output/test-verbose-yes | 4 ++--
>  4 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/test/T000-basic.sh b/test/T000-basic.sh
> index 36a7ca4c..0d6a2081 100755
> --- a/test/T000-basic.sh
> +++ b/test/T000-basic.sh
> @@ -52,7 +52,7 @@ suppress_diff_date() {
>  test_begin_subtest "Ensure that test output is suppressed unless the test fails"
>  output=$(cd $TEST_DIRECTORY; NOTMUCH_TEST_QUIET= ./test-verbose 2>&1 | suppress_diff_date)
>  expected=$(cat $EXPECTED/test-verbose-no | suppress_diff_date)
> -test_expect_equal "$output" "$expected"
> +test_expect_equal "$expected" "$output"
>  
>  test_begin_subtest "Ensure that -v does not suppress test output"
>  output=$(cd $TEST_DIRECTORY; NOTMUCH_TEST_QUIET= ./test-verbose -v 2>&1 | suppress_diff_date)
> @@ -60,7 +60,7 @@ expected=$(cat $EXPECTED/test-verbose-yes | suppress_diff_date)
>  # Do not include the results of test-verbose in totals
>  rm $TEST_DIRECTORY/test-results/test-verbose
>  rm -r $TEST_DIRECTORY/tmp.test-verbose
> -test_expect_equal "$output" "$expected"
> +test_expect_equal "$expected" "$output"
>  
>  
>  ################################################################
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 988b00af..374f6da5 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -566,8 +566,9 @@ test_expect_equal ()
>  	test "$#" = 2 ||
>  	error "bug in the test script: not 2 parameters to test_expect_equal"
>  
> -	output="$1"
> -	expected="$2"
> +	expected="$1"
> +	output="$2"
> +
>  	if ! test_skip "$test_subtest_name"
>  	then
>  		if [ "$output" = "$expected" ]; then
> diff --git a/test/test.expected-output/test-verbose-no b/test/test.expected-output/test-verbose-no
> index 1a2ff619..07004182 100644
> --- a/test/test.expected-output/test-verbose-no
> +++ b/test/test.expected-output/test-verbose-no
> @@ -14,8 +14,8 @@ hello stderr
>  	--- test-verbose.4.expected	2010-11-14 21:41:12.738189710 +0000
>  	+++ test-verbose.4.output	2010-11-14 21:41:12.738189710 +0000
>  	@@ -1 +1 @@
> -	-b
> -	+a
> +	-a
> +	+b
>  hello stdout
>  hello stderr
>  
> diff --git a/test/test.expected-output/test-verbose-yes b/test/test.expected-output/test-verbose-yes
> index d25466e9..639b7fa7 100644
> --- a/test/test.expected-output/test-verbose-yes
> +++ b/test/test.expected-output/test-verbose-yes
> @@ -20,6 +20,6 @@ hello stderr
>  	--- test-verbose.4.expected	2010-11-14 21:41:06.650023289 +0000
>  	+++ test-verbose.4.output	2010-11-14 21:41:06.650023289 +0000
>  	@@ -1 +1 @@
> -	-b
> -	+a
> +	-a
> +	+b
>  
> -- 
> 2.11.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

  reply	other threads:[~2017-04-06 16:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-05  0:36 [PATCH 1/2] test: standardize argument order to test_expect_equal_file David Bremner
2017-04-05  0:36 ` [PATCH 2/2] test: remove debugging "output" David Bremner
2017-04-06  1:12   ` [PATCH] test: swap order of arguments of test_expect_equal David Bremner
2017-04-06 16:24     ` Tomi Ollila [this message]
2017-04-06 17:40       ` David Bremner
2017-04-05  6:15 ` [PATCH 1/2] test: standardize argument order to test_expect_equal_file Tomi Ollila

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://notmuchmail.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m2pogpwlki.fsf@guru.guru-group.fi \
    --to=tomi.ollila@iki.fi \
    --cc=david@tethera.net \
    --cc=notmuch@notmuchmail.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).