Re: [PATCH 2/2] Unset CURLOPT_FAILONERROR

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 28, 2018, 19:58 UTC
Message-ID: <CAPig+cTOK9XkGiPcHdVrWbV-L0a+Av0Kv=NcRVYsUHnb0j5-Zw@mail.gmail.com>
In-Reply-To: <CAJB1erXq3JZMd9XwUZFHp80Hr2kRakrp3JJgrAugXPyjp1rxNg@mail.gmail.com>

On Fri, Dec 28, 2018 at 2:51 PM Masaya Suzuki <masayasuzuki@google.com> wrote:

Show 16 quoted lines

> On Fri, Dec 28, 2018 at 11:37 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Thu, Dec 27, 2018 at 8:47 PM Masaya Suzuki <masayasuzuki@google.com> wrote:
> > > +test_expect_success 'failure in git-upload-pack is shown' '
> > > +       (GIT_CURL_VERBOSE=1 git clone --bare "$HTTPD_URL/error_git_upload_pack/smart/repo.git" 2>curl_log ||
> > > +        true) &&
> >
> > Using test_might_fail() would allow you to drop the subshell and the "|| true":
> >
> >     test_might_fail env GIT_CURL_VERBOSE=1  git clone ... &&
> >
> > > +       cat curl_log | grep "< HTTP/1.1 500 Intentional Breakage"
> > > +'
>
> The test should success. This is a test that a log is produced after a
> git command fails. The point of this test is "cat curl_log | grep ..."
> part that asserts the log.

Unfortunately, the name "test_might_fail" is confusing. It is not saying that the entire test might or might not fail. Rather, it is saying that the one command might or might not fail (and that you don't care if it does fail). The idiom:

    (some-git-command || true) &&

can be replaced with:

   test_might_fail some-git-command &&

without changing its meaning, and without affecting the success/failure status of the test overall.

So, this new test could be written like this:

--- 8< ---
test_expect_success 'failure in git-upload-pack is shown' '
   test_might_fail env GIT_CURL_VERBOSE=1 git clone --bare
"$HTTPD_URL/error_git_upload_pack/smart/repo.git" 2>curl_log &&
   cat curl_log | grep "< HTTP/1.1 500 Intentional Breakage"
'
--- 8< ---

and have the same meaning.