git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Aaron Lipman <alipman88@gmail.com>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [PATCH v3 1/3] t3201: test multiple branch filter combinations
Date: Sun, 13 Sep 2020 16:58:14 -0400
Message-ID: <CAPig+cRrgC5vz9S1qDpEjoEvniBLCbuWt=bkpa2fj=suUm3Bog@mail.gmail.com> (raw)
In-Reply-To: <20200913193140.66906-2-alipman88@gmail.com>

On Sun, Sep 13, 2020 at 3:32 PM Aaron Lipman <alipman88@gmail.com> wrote:
> Add tests covering the behavior of passing multiple contains/no-contains
> filters to git branch, e.g.:
> ---
> diff --git a/t/t3201-branch-contains.sh b/t/t3201-branch-contains.sh
> @@ -200,15 +200,51 @@ test_expect_success 'branch --merged with --verbose' '
> +# The next series of tests covers multiple filter combinations
> +test_expect_success 'set up repo for multiple filter combination tests' '
> +       git checkout master &&
> +       git branch | grep -v master | xargs git branch -D &&
> +       git checkout -b feature_a master &&
> +       >feature_a &&
> +       git add feature_a &&
> +       git commit -m "add feature a" &&
> +       git checkout -b feature_b master &&
> +       >feature_b &&
> +       git add feature_b &&
> +       git commit -m "add feature b"
> +'

A few comments:

I didn't examine it too closely, so this may be a silly question, but
is there a reason to start from scratch (by deleting all the branches)
rather than simply using or extending the existing branches like the
other tests do?

If it really does make sense to start from scratch (ignoring the
existing branches), then an alternative would be to create a new
repository and run the tests in that repository instead. Whether or
not doing so makes sense in this case is a judgment call. For
instance:

    test_create_repo features
    (
        cd features
        ...setup stuff...
    )

It's a bit concerning to see output from porcelain git-branch being
fed to 'grep' and 'xargs'. More typically, you would instead rely upon
the (stable) output of a plumbing command. For instance:

    git for-each-ref --format="%(refname:short)" refs/heads/ | ...

In new test code, normally avoid having a Git command upstream of a
pipe since its exit code will be lost. Thus, you might instead write:

    git for-each-ref ... >heads &&
    grep -v master heads | xargs git branch -D &&

  reply	other threads:[~2020-09-13 20:58 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-08 15:37 [PATCH] ref-filter: allow merged and no-merged filters Aaron Lipman
2020-09-08 22:46 ` Junio C Hamano
2020-09-08 23:57   ` Aaron Lipman
2020-09-09  0:00     ` Junio C Hamano
2020-09-11 18:57 ` [PATCH v2 0/2] git branch: allow combining " Aaron Lipman
2020-09-11 18:57   ` [PATCH v2 1/2] t3201: test multiple branch filter combinations Aaron Lipman
2020-09-11 18:57   ` [PATCH v2 2/2] ref-filter: allow merged and no-merged filters Aaron Lipman
2020-09-11 21:47     ` Junio C Hamano
2020-09-13 19:31     ` [PATCH v3 0/3] git branch: allow combining " Aaron Lipman
2020-09-13 19:31       ` [PATCH v3 1/3] t3201: test multiple branch filter combinations Aaron Lipman
2020-09-13 20:58         ` Eric Sunshine [this message]
2020-09-14 20:07           ` Junio C Hamano
2020-09-13 19:31       ` [PATCH v3 2/3] Doc: cover multiple contains/no-contains filters Aaron Lipman
2020-09-13 21:10         ` Eric Sunshine
2020-09-14 20:07           ` Junio C Hamano
2020-09-13 19:31       ` [PATCH v3 3/3] ref-filter: allow merged and no-merged filters Aaron Lipman
2020-09-13 21:36         ` Eric Sunshine
2020-09-13 21:56           ` Junio C Hamano
2020-09-13 22:31             ` Eric Sunshine
2020-09-16  2:08       ` [PATCH v4 0/3] git branch: allow combining " Aaron Lipman
2020-09-16  2:08         ` [PATCH v4 1/3] t3201: test multiple branch filter combinations Aaron Lipman
2020-09-16  2:08         ` [PATCH v4 2/3] Doc: cover multiple contains/no-contains filters Aaron Lipman
2020-09-16 19:45           ` Junio C Hamano
2020-09-16  2:08         ` [PATCH v4 3/3] ref-filter: allow merged and no-merged filters Aaron Lipman
2020-09-16  4:53         ` [PATCH v4 0/3] git branch: allow combining " Junio C Hamano
2020-09-16  5:08           ` Eric Sunshine
2020-09-18  0:49         ` [PATCH 0/2] ref-filter: merged & no-merged touch-ups Aaron Lipman
2020-09-18  0:49           ` [PATCH 1/2] ref-filter: refactor do_merge_filter() Aaron Lipman
2020-09-18  5:03             ` Eric Sunshine
2020-09-18  5:04               ` Eric Sunshine
2020-09-18 17:01               ` Junio C Hamano
2020-09-18  0:49           ` [PATCH 2/2] Doc: prefer more specific file name Aaron Lipman
2020-09-18  2:52           ` [PATCH 0/2] ref-filter: merged & no-merged touch-ups Eric Sunshine
2020-09-18 21:58           ` [PATCH v2 " Aaron Lipman
2020-09-18 21:58             ` [PATCH v2 1/2] ref-filter: make internal reachable-filter API more precise Aaron Lipman
2020-09-18 21:58             ` [PATCH v2 2/2] Doc: prefer more specific file name Aaron Lipman

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: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to='CAPig+cRrgC5vz9S1qDpEjoEvniBLCbuWt=bkpa2fj=suUm3Bog@mail.gmail.com' \
    --to=sunshine@sunshineco.com \
    --cc=alipman88@gmail.com \
    --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

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git