Re: [PATCH v3 1/3] t3201: test multiple branch filter combinations

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 13, 2020, 20:58 UTC
Message-ID: <CAPig+cRrgC5vz9S1qDpEjoEvniBLCbuWt=bkpa2fj=suUm3Bog@mail.gmail.com>
In-Reply-To: <20200913193140.66906-2-alipman88@gmail.com>

On Sun, Sep 13, 2020 at 3:32 PM Aaron Lipman <alipman88@gmail.com> wrote:

Show 18 quoted lines

> 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 &&