Re: [PATCH v3 1/6] t9001: non order-sensitive file comparison

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Tom Russello <tom.russello@xxxxxxxxxxxxxxxx> writes:

> +# Check if two files have the same content, non-order sensitive
> +test_cmp_noorder () {
> +	sort $1 >$1;

Here is what I think happens:

    0) the shell parses this command line;
    1) the shell notices that the output has to go to $1;
    2) the shell does open(2) of $1,
    3) the shell spawns "sort" with a single argument, with its
       output connected to the file descriptor obtained in 2).

Because "$1" becomes an empty file at 2), "sort" reads nothing and
writes nothing.

> +	sort $2 >$2;
> +	return $(test_cmp $1 $2)

What is this return doing?  I would understand if it were just

	test_cmp $1 $2

Of course, all the places you use test_cmp_noorder are happy when
this function returns 0/success, and because $1 and $2 at this point
are both empty files and test_cmp will not say anything to its
standard output, the return will just yield 0/success to the caller
of the function, so it is likely that with this patch t9001 would
have passed for you, but that is not necessarily a good thing X-<.

> @@ -269,7 +276,7 @@ test_expect_success $PREREQ 'Show all headers' '
>  		-e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
>  		-e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \
>  		>actual-show-all-headers &&
> -	test_cmp expected-show-all-headers actual-show-all-headers
> +	test_cmp_noorder expected-show-all-headers actual-show-all-headers
>  '

It is dubious that it is a good idea to blindly sort two files and
compare, especially because expected-show-all-headers is actually
something like this:

    cat >expected-show-all-headers <<\EOF
    0001-Second.patch
    (mbox) Adding cc: A <author@xxxxxxxxxxx> from line 'From: A <author@xxxxxxxxxxx>'
    (mbox) Adding cc: One <one@xxxxxxxxxxx> from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx'
    (mbox) Adding cc: two@xxxxxxxxxxx from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx'
    Dry-OK. Log says:
    Server: relay.example.com
    MAIL FROM:<from@xxxxxxxxxxx>
    RCPT TO:<to@xxxxxxxxxxx>
    RCPT TO:<cc@xxxxxxxxxxx>
    ...
    To: to@xxxxxxxxxxx
    Cc: cc@xxxxxxxxxxx,
            A <author@xxxxxxxxxxx>,
            One <one@xxxxxxxxxxx>,
            two@xxxxxxxxxxx
    Subject: [PATCH 1/1] Second.
    Date: DATE-STRING
    Message-Id: MESSAGE-ID-STRING
    X-Mailer: X-MAILER-STRING
    In-Reply-To: <unique-message-id@xxxxxxxxxxx>
    References: <unique-message-id@xxxxxxxxxxx>

    Result: OK
    EOF

We do want to see MAIL FROM: as the first thing we give to the
server, followed by RCPT TO:, followed by the headers.

I am having a hard time guessing what prompted you to sort the
output, i.e. what problem you were trying to solve.  It cannot be
because addresses on a list (e.g. Cc:) could come out in an
indeterministic order, because the address that a test expects to be
the first (cc@xxxxxxxxxxx in the above example) may not appear as
the first one, but in the textual output it _is_ shown differently
from the remainder (i.e. even if you sort, from "Cc:
cc@xxxxxxxxxxx," it is clear it was the first one output for Cc: and
diferent from "A <author@xxxxxxxxxxx>".

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]