From: Junio C Hamano <gitster@pobox.com>
To: applehq <theappleman@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] t9129: Prevent test failure if no UTF-8 locale
Date: Fri, 05 Dec 2008 18:05:50 -0800 [thread overview]
Message-ID: <7vk5aevz0h.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <20081206013152.GA6129@cumin> (theappleman@gmail.com's message of "Sat, 6 Dec 2008 01:31:52 +0000")
applehq <theappleman@gmail.com> writes:
> Commit 16fc08e2d86dad152194829d21bc55b2ef0c8fb1 introduced a
> test that failed if the en_US.UTF-8 locale was not installed.
>
> Make the test find a UTF-8 locale, and expect failure.
NAK on the latter one.
test_expect_failure does not mean "This might, and it is Ok to, fail", as
you seem to think. It means "This should succeed if our software is not
buggy, but there is a known breakage to cause it to fail, so this test is
marked as such until the bug is fixed."
Skipping the test on a platform that lacks necessary locale is fine, but
if you run the test, they should expect success. Otherwise you cannot
tell if it is a platform issue (i.e. not having any UTF-8 locale) or
a bug in the software (i.e. git-svn not working as expected).
> Signed-off-by: applehq <theappleman@gmail.com>
Sign off with an unreal name, hmm..., what good would that give us...?
> @@ -15,8 +15,9 @@ compare_git_head_with () {
> }
>
> compare_svn_head_with () {
> - LC_ALL=en_US.UTF-8 svn log --limit 1 `git svn info --url` | \
> - sed -e 1,3d -e "/^-\{1,\}\$/d" >current &&
> + LC_ALL=`locale -a | grep -i utf | head -1` \
> + svn log --limit 1 `git svn info --url` | \
> + sed -e 1,3d -e "/^-\{1,\}\$/d" >current &&
I think what this part tries to do is good, in that we do not care if it
is en_US UTF-8 or any other UTF-8. But cramming that logic all in one
pipeline (and an inefficient one at that) makes it harder to read.
Do something like this upfront:
a_utf8_locale=$(locale -a | sed -ne '/[Uu][Tt][Ff]-*8/{
p
q
'})
and if that variable is empty then skip the test that relies on having a
UTF-8 locale on the platform. Then the function can say:
LC_ALL="$a_utf8_locale" svn log --limit 1 ...
prev parent reply other threads:[~2008-12-06 2:08 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-12-06 1:31 [PATCH] t9129: Prevent test failure if no UTF-8 locale applehq
2008-12-06 2:05 ` Junio C Hamano [this message]
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vk5aevz0h.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=theappleman@gmail.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.
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 NNTP newsgroup(s).