Re: [BUG] t7004 (master) busted on Leopard

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



El 15/11/2007, a las 15:37, Johannes Schindelin escribió:

Hi,

On Thu, 15 Nov 2007, Wincent Colaiuta wrote:

Commit 4d8b1dc850 added a couple of tests to t7004, and my testing reveals
that this one has been broken on Leopard since then:

* FAIL 83: message in editor has initial comment
GIT_EDITOR=cat git tag -a initial-comment > actual || true &&
test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0

I think this is our good old friend, MacOSX' sed.

Yes, that's exactly what I said in the part of my post that you didn't quote.

I imagine that it is that MacOSX' sed is adding a trailing newline (not the regexp like you suggested). Which means that "wc -l" would print "1".
(You can see for yourself if you run the script with "sh -x ...".)

Unless I am misreading the test, any output from "wc -l" that is greater than 0 will cause the test to pass, so even if it outputted "1" as you suggest then that wouldn't be the cause of the failure.

I do think the cause of the failure is the limited regexp syntax that sed accepts on Leopard; witness: on Mac OS X the following prints 0:

  echo "# hello" | sed -n "/^\(#\|\$\)/p" | wc -l

Whereas the following prints 1:

  echo "# hello" | sed -n "/^[#\$]/p" | wc -l

Here's the output for the failing test run under "sh -x" as you suggest, although I must admit that I can't really parse it myself.

+ test_expect_success 'message in editor has initial comment' '
	GIT_EDITOR=cat git tag -a initial-comment > actual || true &&
	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
'
+ test 2 = 2
+ test_skip 'message in editor has initial comment' '
	GIT_EDITOR=cat git tag -a initial-comment > actual || true &&
	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
'
++ expr ./t7004-tag.sh : '.*/\(t[0-9]*\)-[^/]*$'
+ this_test=t7004
++ expr 82 + 1
+ this_test=t7004.83
+ to_skip=
+ case "$to_skip" in
+ false
+ say 'expecting success:
	GIT_EDITOR=cat git tag -a initial-comment > actual || true &&
	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
'
+ say_color info 'expecting success:
	GIT_EDITOR=cat git tag -a initial-comment > actual || true &&
	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
'
+ case "$1" in
+ tput setaf 3
+ shift
+ echo '* expecting success:
	GIT_EDITOR=cat git tag -a initial-comment > actual || true &&
	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
'
+ tput sgr0
+ test_run_ '
	GIT_EDITOR=cat git tag -a initial-comment > actual || true &&
	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
'
+ eval '
	GIT_EDITOR=cat git tag -a initial-comment > actual || true &&
	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
'
+ eval_ret=1
+ return 0
+ '[' 0 = 0 -a 1 = 0 ']'
+ test_failure_ 'message in editor has initial comment' '
	GIT_EDITOR=cat git tag -a initial-comment > actual || true &&
	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
'
++ expr 82 + 1
+ test_count=83
++ expr 0 + 1
+ test_failure=1
+ say_color error 'FAIL 83: message in editor has initial comment'
+ case "$1" in
+ tput bold
+ tput setaf 1
+ shift
+ echo '* FAIL 83: message in editor has initial comment'
* FAIL 83: message in editor has initial comment
+ tput sgr0
+ shift

IMHO a good solution would be

	test -z "$(grep -e '^#' -e '^$' actual)"

Could you test, please?

Yes, the test passes with that, although it has to be written as follows in the actual test file seeing as it's inside a single-quoted string:

test -z "$(grep -e \"^#\" -e \"^$\" actual)"

Will follow this up with a patch that implements your proposed fix.

Cheers,
Wincent


-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux