git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Martin Ågren" <martin.agren@gmail.com>
To: "Randall S . Becker" <rsbecker@nexbridge.com>
Cc: Derrick Stolee <dstolee@microsoft.com>, git@vger.kernel.org
Subject: [PATCH] t1091: don't grep for `strerror()` string
Date: Sun,  8 Mar 2020 09:46:27 +0100	[thread overview]
Message-ID: <20200308084627.26677-1-martin.agren@gmail.com> (raw)
In-Reply-To: <018001d5f305$0c709af0$2551d0d0$@nexbridge.com>

We grep for "File exists" in stderr of the failing `git sparse-checkout`
to make sure that it failed for the right reason. We expect the string
to show up there since we call `strerror(errno)` in
`unable_to_lock_message()` in lockfile.c.

On the NonStop platform, this fails because the error string is "File
already exists", which doesn't match our grepping.

See 9042140097 ("test-dir-iterator: do not assume errno values",
2019-07-30) for a somewhat similar fix. There, we patched a test helper,
which meant we had access to `errno` and could investigate it better in
the test helper instead of just outputting the numerical value and
evaluating it in the test script. The current situation is different,
since (short of modifying the lockfile machinery, e.g., to be more
verbose) we don't have more than the output from `strerror()` available.

Except we do: We prefix `strerror(errno)` with `_("Unable to create
'%s.lock': ")`. Let's grep for that part instead. It verifies that we
were indeed unable to create the lock file. (If that fails for some
other reason than the file existing, we really really should expect
other tests to fail as well.)

An alternative fix would be to loosen the expression a bit and grep for
"File.* exists" instead. There would be no guarantee that some other
implementation couldn't come up with another error string, That is, that
could be the first move in an endless game of whack-a-mole. Of course,
it could also take us from "99" to "100" percent of the platforms and
we'd never have this problem again. But since we have another way of
addressing this, let's not even try the "loosen it up a bit" strategy.

Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Hi Randall,

 Thanks for the report.

 > test to fail. The error message generated is "File already exists" not "File
 > exists" as is required in the test. We should not be testing for specific
 > text content originating from strerror - I thought we had this decision in a
 > different thread.
 > https://public-inbox.org/git/xmqq36intlpj.fsf@gitster-ct.c.googlers.com/

 > error: 'grep File exists err' didn't find a match in:
 > fatal: Unable to create '/home/ituglib/randall/git/t/trash
 > directory.t1091-sparse-checkout-builtin/repo/.git/info/sparse-checkout.lock'
 > : File already exists.   <----- this is the test issue

 Does this patch solve it?

 t/t1091-sparse-checkout-builtin.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index b4c9c32a03..44a91205d6 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@ -305,7 +305,7 @@ test_expect_success 'fail when lock is taken' '
 	test_when_finished rm -rf repo/.git/info/sparse-checkout.lock &&
 	touch repo/.git/info/sparse-checkout.lock &&
 	test_must_fail git -C repo sparse-checkout set deep 2>err &&
-	test_i18ngrep "File exists" err
+	test_i18ngrep "Unable to create .*\.lock" err
 '
 
 test_expect_success '.gitignore should not warn about cone mode' '
-- 
2.25.1


  reply	other threads:[~2020-03-08  8:46 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-05 15:45 [Test] t1901 - sparse checkout file when lock is taken fails (subtest 19) Randall S. Becker
2020-03-08  8:46 ` Martin Ågren [this message]
2020-03-09 13:07   ` [PATCH] t1091: don't grep for `strerror()` string Derrick Stolee
2020-03-09 15:04   ` Junio C Hamano

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=20200308084627.26677-1-martin.agren@gmail.com \
    --to=martin.agren@gmail.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=rsbecker@nexbridge.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).