All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Kristoffer Haugsbakk <code@khaugsbakk.name>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v3 1/5] t3200: improve test style
Date: Mon, 04 Mar 2024 17:25:28 -0800	[thread overview]
Message-ID: <xmqqplw9lbav.fsf@gitster.g> (raw)
In-Reply-To: <e6a2628ce57668aa17101e73edaead0ef34d8a8c.1709590037.git.code@khaugsbakk.name> (Kristoffer Haugsbakk's message of "Mon, 4 Mar 2024 23:07:26 +0100")

Kristoffer Haugsbakk <code@khaugsbakk.name> writes:

> Also:
>
> • Remove a now-irrelevant comment about test placement and switch back
>   to `main` post-test.
> • Prefer indented literal heredocs (`-\EOF`) except for a block which
>   says that this is intentional
> • Move a `git config` command into the test and mark it as `setup` since
>   the next test depends on it
>

Especially the change to use "-\EOF" to make them align better
caused too many tests to be touched, but overall the result may have
become much easier to follow.  Good job.

> -mv .git/config .git/config-saved
> -
>  test_expect_success DEFAULT_REPO_FORMAT 'git branch -m q q2 without config should succeed' '
> +	test_when_finished mv .git/config-saved .git/config &&
> +	mv .git/config .git/config-saved &&
>  	git branch -m q q2 &&
>  	git branch -m q2 q
>  '
>  
> -mv .git/config-saved .git/config

The above is a truly valuable clean-up.

But I am not really sure if the paritcular condition is worth
testing in the first place these days.  No configuration file means
we cannot even read the repository format version, and working under
such a condition is quite a bad promise that we would rather not to
having to keep.  But that is an entirely different topic from what
this patch is doing.

> -git config branch.s/s.dummy Hello
> -
> -test_expect_success 'git branch -m s/s s should work when s/t is deleted' '
> +test_expect_success '(setup) git branch -m s/s s should work when s/t is deleted' '
> +	git config branch.s/s.dummy Hello &&
>  	git branch --create-reflog s/s &&
>  	git reflog exists refs/heads/s/s &&
>  	git branch --create-reflog s/t &&

I do not know if the change of the title is warranted.  It is doing
its own test, not just setup.  It may be merely donw for the side
effect of making the step unskippable, but still ....

> -# Keep this test last, as it changes the current branch

Yes, removal of this line is really appreciated ;-)

> -cat >expect <<EOF
> -$HEAD refs/heads/g/h/i@{0}: branch: Created from main
> -EOF
>  test_expect_success 'git checkout -b g/h/i -l should create a branch and a log' '
> +	test_when_finished git checkout main &&
>  	GIT_COMMITTER_DATE="2005-05-26 23:30" \
>  	git checkout -b g/h/i -l main &&
>  	test_ref_exists refs/heads/g/h/i &&
> +	cat >expect <<-EOF &&
> +	$HEAD refs/heads/g/h/i@{0}: branch: Created from main
> +	EOF
>  	git reflog show --no-abbrev-commit refs/heads/g/h/i >actual &&
>  	test_cmp expect actual
>  '

Thanks.

  reply	other threads:[~2024-03-05  1:25 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-01 15:38 [PATCH] branch: advise about ref syntax rules Kristoffer Haugsbakk
2024-03-01 18:06 ` Junio C Hamano
2024-03-01 18:13   ` Kristoffer Haugsbakk
2024-03-01 18:32     ` Junio C Hamano
2024-03-03 18:58 ` [PATCH v2 0/1] " Kristoffer Haugsbakk
2024-03-03 18:58   ` [PATCH v2 1/1] branch: " Kristoffer Haugsbakk
2024-03-03 22:42     ` Junio C Hamano
2024-03-03 22:58       ` Kristoffer Haugsbakk
2024-03-04 22:07     ` [PATCH v3 0/5] " Kristoffer Haugsbakk
2024-03-04 22:07       ` [PATCH v3 1/5] t3200: improve test style Kristoffer Haugsbakk
2024-03-05  1:25         ` Junio C Hamano [this message]
2024-03-05 10:27           ` Kristoffer Haugsbakk
2024-03-05 16:02             ` Junio C Hamano
2024-03-04 22:07       ` [PATCH v3 2/5] advice: make all entries stylistically consistent Kristoffer Haugsbakk
2024-03-04 23:52         ` Junio C Hamano
2024-03-05 10:36           ` Kristoffer Haugsbakk
2024-03-04 22:07       ` [PATCH v3 3/5] advice: use backticks for code Kristoffer Haugsbakk
2024-03-04 23:54         ` Junio C Hamano
2024-03-05 10:29           ` Kristoffer Haugsbakk
2024-03-04 22:07       ` [PATCH v3 4/5] advice: use double quotes for regular quoting Kristoffer Haugsbakk
2024-03-04 22:07       ` [PATCH v3 5/5] branch: advise about ref syntax rules Kristoffer Haugsbakk
2024-03-05 20:29       ` [PATCH v4 0/5] " Kristoffer Haugsbakk
2024-03-05 20:29         ` [PATCH v4 1/5] t3200: improve test style Kristoffer Haugsbakk
2024-03-05 20:29         ` [PATCH v4 2/5] advice: make all entries stylistically consistent Kristoffer Haugsbakk
2024-03-05 20:29         ` [PATCH v4 3/5] advice: use backticks for verbatim Kristoffer Haugsbakk
2024-03-05 20:29         ` [PATCH v4 4/5] advice: use double quotes for regular quoting Kristoffer Haugsbakk
2024-03-05 20:29         ` [PATCH v4 5/5] branch: advise about ref syntax rules Kristoffer Haugsbakk
2024-03-03 19:10   ` [PATCH v2 0/1] " Kristoffer Haugsbakk

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=xmqqplw9lbav.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=code@khaugsbakk.name \
    --cc=git@vger.kernel.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.
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.