All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jacob Abel <jacobabel@nullpo.dev>
Cc: git@vger.kernel.org, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Rubén Justo" <rjusto@gmail.com>, "Taylor Blau" <me@ttaylorr.com>,
	rsbecker@nexbridge.com
Subject: Re: [PATCH v9 2/8] t2400: print captured git output when finished
Date: Mon, 17 Apr 2023 14:09:52 -0700	[thread overview]
Message-ID: <xmqq8reqkyfz.fsf@gitster.g> (raw)
In-Reply-To: <20230417093255.31079-3-jacobabel@nullpo.dev> (Jacob Abel's message of "Mon, 17 Apr 2023 09:33:44 +0000")

Jacob Abel <jacobabel@nullpo.dev> writes:

>  test_expect_success 'add --quiet' '
> +	test_when_finished "git worktree remove -f -f another-worktree" &&
> +	test_when_finished cat actual >&2 &&

I doubt that this redirection does anything you expect it do.
Doesn't it redirect the standard output that is emitted by the
test_when_finished shell function when it registers another
test_cleanup scriptlet to the standard error, and when test_cleanup
is indeed run, wouldn't "cat actual" send its output to the standard
output?

No, I am not suggesting to write the line as:

	test_when_finished "cat >&2 actual" &&

>  	git worktree add --quiet another-worktree main 2>actual &&
>  	test_must_be_empty actual

The reason why I do not suggest "fixing" the above is because
test_must_be_empty, when fails, does this:

        test_must_be_empty () {
                test "$#" -ne 1 && BUG "1 param"
                test_path_is_file "$1" &&
                if test -s "$1"
                then
                        echo "'$1' is not empty, it contains:"
                        cat "$1"
                        return 1
                fi
        }

i.e. it sends the contents of "actual" to the standard output
already.  When it succeeds, of course "actual" is empty, and there
is no point in showing its contents.

So "sh t2400-*.sh -x -i" already shows "cat actual" output.  Try
the attached patch on top of this one and running it would show
the above message shown by test_must_be_empty and the contents of
the file 'actual'.  "git worktree remove" fails and your "cat" in
the test_cleanup does not even trigger, by the way.

There may be cases where having something like this might help, but
running the test with "-x" is not it---that case is already covered
by what test_must_be_empty gives us, I think.

 t/t2400-worktree-add.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git c/t/t2400-worktree-add.sh w/t/t2400-worktree-add.sh
index 9bc3db20e4..814642c8ae 100755
--- c/t/t2400-worktree-add.sh
+++ w/t/t2400-worktree-add.sh
@@ -329,9 +329,12 @@ test_expect_success 'add --quiet' '
 	test_when_finished "git worktree remove -f -f another-worktree" &&
 	test_when_finished cat actual >&2 &&
 	git worktree add --quiet another-worktree main 2>actual &&
+echo foo >>actual &&
 	test_must_be_empty actual
 '
 
+exit
+
 test_expect_success 'local clone from linked checkout' '
 	git clone --local here here-clone &&
 	( cd here-clone && git fsck )


  reply	other threads:[~2023-04-17 21:10 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-17  9:33 [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
2023-04-17  9:33 ` [PATCH v9 1/8] worktree add: include -B in usage docs Jacob Abel
2023-04-17  9:33 ` [PATCH v9 2/8] t2400: print captured git output when finished Jacob Abel
2023-04-17 21:09   ` Junio C Hamano [this message]
2023-04-18  3:53     ` Jacob Abel
2023-04-18 16:34       ` Junio C Hamano
2023-04-19 13:23         ` Jacob Abel
2023-04-19 13:36         ` Jacob Abel
2023-04-19 15:41           ` Junio C Hamano
2023-04-19 16:50             ` Jacob Abel
2023-04-17  9:33 ` [PATCH v9 3/8] t2400: refactor "worktree add" opt exclusion tests Jacob Abel
2023-04-17 21:30   ` Junio C Hamano
2023-04-20  2:46     ` Jacob Abel
2023-04-17  9:34 ` [PATCH v9 4/8] t2400: add tests to verify --quiet Jacob Abel
2023-04-17 21:33   ` Junio C Hamano
2023-04-20  2:48     ` Jacob Abel
2023-04-17  9:34 ` [PATCH v9 5/8] worktree add: add --orphan flag Jacob Abel
2023-04-17  9:34 ` [PATCH v9 6/8] worktree add: introduce "try --orphan" hint Jacob Abel
2023-04-17  9:34 ` [PATCH v9 7/8] worktree add: extend DWIM to infer --orphan Jacob Abel
2023-04-17  9:34 ` [PATCH v9 8/8] worktree add: emit warn when there is a bad HEAD Jacob Abel
2023-04-20  3:05 ` [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
2023-05-01 21:51   ` Junio C Hamano
2023-05-02  5:48     ` Jacob Abel
2023-05-17 21:47 ` [RESEND PATCH v10 " Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 1/8] worktree add: include -B in usage docs Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 2/8] t2400: cleanup created worktree in test Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 3/8] t2400: refactor "worktree add" opt exclusion tests Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 4/8] t2400: add tests to verify --quiet Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 5/8] worktree add: add --orphan flag Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 6/8] worktree add: introduce "try --orphan" hint Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 7/8] worktree add: extend DWIM to infer --orphan Jacob Abel
2023-08-09  6:47     ` RESEND [PATCH " Teng Long
2023-08-11 17:43       ` Jacob Abel
2023-05-17 21:49   ` [RESEND PATCH v10 8/8] worktree add: emit warn when there is a bad HEAD Jacob Abel

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

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

  git send-email \
    --in-reply-to=xmqq8reqkyfz.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jacobabel@nullpo.dev \
    --cc=me@ttaylorr.com \
    --cc=phillip.wood123@gmail.com \
    --cc=rjusto@gmail.com \
    --cc=rsbecker@nexbridge.com \
    --cc=sunshine@sunshineco.com \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.