Web lists-archives.com

Re: [PATCH 2/2] Unset CURLOPT_FAILONERROR




On Fri, Dec 28, 2018 at 2:51 PM Masaya Suzuki <masayasuzuki@xxxxxxxxxx> wrote:
> On Fri, Dec 28, 2018 at 11:37 AM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> > On Thu, Dec 27, 2018 at 8:47 PM Masaya Suzuki <masayasuzuki@xxxxxxxxxx> 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.