From: Eric Sunshine <sunshine@sunshineco.com>
Date: Fri, 02 Dec 2022 01:10:47 GMT
Subject: Re: [PATCH v2 2/8] auto-crlf tests: check "git checkout" exit code
Message-ID: <CAPig+cRwwyiQdbAhDYGH==8r+Q2HVuqbW2a8c6W_myyqKAs4eA@mail.gmail.com>
In-Reply-To: <02ead668-5b5d-877c-1e40-3656fd85945f@web.de>

On Thu, Dec 1, 2022 at 8:02 PM René Scharfe <l.s.r@web.de> wrote:
> Am 02.12.2022 um 01:06 schrieb Ævar Arnfjörð Bjarmason:
> > Don't hide the exit code from the "git checkout" we run to checkout
> > our attributes file.
> > @@ -294,11 +294,17 @@ checkout_files () {
> >       pfx=eol_${ceol}_crlf_${crlf}_attr_${attr}_ &&
> >       for f in LF CRLF LF_mix_CR CRLF_mix_LF LF_nul
> >       do
> > -             rm crlf_false_attr__$f.txt &&
> > -             if test -z "$ceol"; then
> > -                     git checkout -- crlf_false_attr__$f.txt
> > +             if test -z "$ceol"
> > +             then
> > +                     test_expect_success "setup $f checkout" '
> > +                             rm crlf_false_attr__$f.txt &&
> > +                             git checkout -- crlf_false_attr__$f.txt
> > +                     '
> >               else
> > -                     git -c core.eol=$ceol checkout -- crlf_false_attr__$f.txt
> > +                     test_expect_success "setup $f checkout with core.eol=$ceol" '
> > +                             rm crlf_false_attr__$f.txt &&
> > +                             git -c core.eol=$ceol checkout -- crlf_false_attr__$f.txt
> > +                     '
>
> That adds five test_expect_success calls.  Wouldn't one suffice, for the
> whole for loop, and a "|| return 1"?

Seems like a reasonable idea.

> One line above the context there's a "git config" call that should also
> be covered, right?

Also, the call to create_gitattributes() just above that line is in
the function's &&-chain, which implies that it too should be covered.