Re: [PATCH v2] Fix git-tag test breakage caused by broken sed on Leopard

8 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v2] Fix git-tag test breakage caused by broken sed on Leopard

From: Wincent Colaiuta <hidden>
Date: 2016-06-15 22:43:52

El 16/11/2007, a las 6:14, Junio C Hamano escribió:
Wincent Colaiuta [off-list ref] writes:
quoted
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 096fe33..b54c2e0 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1007,7 +1007,7 @@ test_expect_failure \
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 $(grep -e "^#" -e "^\$" actual | wc -l ) -gt 0
'
Heh, doesn't grep exit with zero only when it found some lines
that match the pattern already?  What's that "wc -l" for?

I was just trying to make the minimal change (swapping grep for sed),  
but if you want a shorter version then we don't even need the "test";  
it could just be:

-	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
+	grep -e "^#" -e "^\$" actual

Cheers,
Wincent

Re: [PATCH v2] Fix git-tag test breakage caused by broken sed on Leopard

From: Wincent Colaiuta <hidden>
Date: 2016-06-15 22:43:52

El 16/11/2007, a las 14:45, Wincent Colaiuta escribió:
El 16/11/2007, a las 6:14, Junio C Hamano escribió:
quoted
Wincent Colaiuta [off-list ref] writes:
quoted
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 096fe33..b54c2e0 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1007,7 +1007,7 @@ test_expect_failure \
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 $(grep -e "^#" -e "^\$" actual | wc -l ) -gt 0
'
Heh, doesn't grep exit with zero only when it found some lines
that match the pattern already?  What's that "wc -l" for?

I was just trying to make the minimal change (swapping grep for  
sed), but if you want a shorter version then we don't even need the  
"test"; it could just be:

-	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
+	grep -e "^#" -e "^\$" actual
Although I don't know if we should be testing for empty lines there  
because an 0-byte empty "actual" file would spuriously pass the test.  
Perhaps this would be better:

-	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
+	grep -e "^#" actual

Cheers,
Wincent

Re: [PATCH v2] Fix git-tag test breakage caused by broken sed on Leopard

From: Mike Hommey <hidden>
Date: 2016-06-15 22:43:52

On Fri, Nov 16, 2007 at 02:48:09PM +0100, Wincent Colaiuta wrote:
El 16/11/2007, a las 14:45, Wincent Colaiuta escribió:
quoted
El 16/11/2007, a las 6:14, Junio C Hamano escribió:
quoted
Wincent Colaiuta [off-list ref] writes:
quoted
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 096fe33..b54c2e0 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1007,7 +1007,7 @@ test_expect_failure \
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 $(grep -e "^#" -e "^\$" actual | wc -l ) -gt 0
'
Heh, doesn't grep exit with zero only when it found some lines
that match the pattern already?  What's that "wc -l" for?

I was just trying to make the minimal change (swapping grep for  
sed), but if you want a shorter version then we don't even need the  
"test"; it could just be:

-	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
+	grep -e "^#" -e "^\$" actual
Although I don't know if we should be testing for empty lines there  
because an 0-byte empty "actual" file would spuriously pass the test.  
Perhaps this would be better:

-	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
+	grep -e "^#" actual
Matching both would as in your previous pseudo patch wouldn't catch
empty file. On the other hand, both my initial bloated version and yours
won't catch a file that doesn't contain the comment.

grep -e "^$" actual && grep -e "^#" actual would actually be a better
test.

Mike

Re: [PATCH v2] Fix git-tag test breakage caused by broken sed on Leopard

From: Wincent Colaiuta <hidden>
Date: 2016-06-15 22:43:52

El 16/11/2007, a las 17:59, Mike Hommey escribió:
On Fri, Nov 16, 2007 at 02:48:09PM +0100, Wincent Colaiuta wrote:
quoted
El 16/11/2007, a las 14:45, Wincent Colaiuta escribió:
quoted
El 16/11/2007, a las 6:14, Junio C Hamano escribió:
quoted
Wincent Colaiuta [off-list ref] writes:
quoted
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 096fe33..b54c2e0 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1007,7 +1007,7 @@ test_expect_failure \
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 $(grep -e "^#" -e "^\$" actual | wc -l ) -gt 0
'
Heh, doesn't grep exit with zero only when it found some lines
that match the pattern already?  What's that "wc -l" for?

I was just trying to make the minimal change (swapping grep for
sed), but if you want a shorter version then we don't even need the
"test"; it could just be:

-	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
+	grep -e "^#" -e "^\$" actual
Although I don't know if we should be testing for empty lines there
because an 0-byte empty "actual" file would spuriously pass the test.
Perhaps this would be better:

-	test $(sed -n "/^\(#\|\$\)/p" actual | wc -l) -gt 0
+	grep -e "^#" actual
Matching both would as in your previous pseudo patch wouldn't catch
empty file. On the other hand, both my initial bloated version and  
yours
won't catch a file that doesn't contain the comment.
But if git-tag works then it will contain a comment; and isn't the  
purpose of the test to confirm the comment's presence?
grep -e "^$" actual && grep -e "^#" actual would actually be a better
test.
What are we really trying to test here? The test is labelled as  
'message in editor has initial comment'. Basically the editor will be  
prepopulated with a blank line followed by this:

#
# Write a tag message
#

So it's the presence of that text which we want to confirm.

If I understand the intent of the original sed-based test, it was to  
confirm that there were 1 or more lines that started with "#" or were  
empty. It also suffered from the bug that an empty message (by which I  
mean a 1-byte file containing only an LF) would pass the test  
(spuriously in my opinion), and my first attempt faithfully translated  
that bug into grep syntax.

The alternative you suggest will (correctly) fail on a zero byte file,  
and pass on 1-byte file containing only a LF, just like the other  
approaches. So I'm still wondering, what is it that we're trying to  
test here? We could test for the exact "Write a tag message" text, but  
that may be brittle (must be updated if/when the text ever changes),  
but looking at other tests I see there are some which do precise  
equality tests for expected results.

Cheers,
Wincent

[PATCH] Fix t7004 which fails with retarded sed

From: Mike Hommey <hidden>
Date: 2016-06-15 22:43:52

Brown paper bag fix to avoid test failure with retarded sed. The test
by itself didn't catch what it was supposed to, anyways.

So now, we test whether the editor gets at least an empty line, some
commented lines, and doesn't get anything else.

Signed-off-by: Mike Hommey <redacted>
---
 t/t7004-tag.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 096fe33..3813f23 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1007,7 +1007,9 @@ test_expect_failure \
 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
+	grep -e "^$" actual > /dev/null 2>&1 &&
+	grep -e "^#" actual > /dev/null 2>&1 &&
+	! grep -e "^[^#]" actual > /dev/null 2>&1
 '
 
 get_tag_header reuse $commit commit $time >expect
-- 
1.5.3.5

Re: [PATCH] Fix t7004 which fails with retarded sed

From: Benoit Sigoure <hidden>
Date: 2016-06-15 22:43:52

On Nov 16, 2007, at 6:26 PM, Mike Hommey wrote:
quoted hunk
Brown paper bag fix to avoid test failure with retarded sed. The test
by itself didn't catch what it was supposed to, anyways.

So now, we test whether the editor gets at least an empty line, some
commented lines, and doesn't get anything else.

Signed-off-by: Mike Hommey <redacted>
---
 t/t7004-tag.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 096fe33..3813f23 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1007,7 +1007,9 @@ test_expect_failure \
 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
+	grep -e "^$" actual > /dev/null 2>&1 &&
+	grep -e "^#" actual > /dev/null 2>&1 &&
+	! grep -e "^[^#]" actual > /dev/null 2>&1
 '

 get_tag_header reuse $commit commit $time >expect
If your system has a retarded `sed', it will most likely also have a  
brain-dead `/bin/sh' which doesn't handle `! command'.  So I suggest  
you rewrite the last line as:
   grep -ve "^[^#]" actual > /dev/null 2>&1

Cheers,

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory

Re: [PATCH] Fix t7004 which fails with retarded sed

From: Wincent Colaiuta <hidden>
Date: 2016-06-15 22:43:52

El 16/11/2007, a las 18:26, Mike Hommey escribió:
quoted hunk
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 096fe33..3813f23 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1007,7 +1007,9 @@ test_expect_failure \
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
+	grep -e "^$" actual > /dev/null 2>&1 &&
+	grep -e "^#" actual > /dev/null 2>&1 &&
+	! grep -e "^[^#]" actual > /dev/null 2>&1
'
Yep, that works here, and is a much more rigourous test than what we  
had.

Cheers,
Wincent

Re: [PATCH] Fix t7004 which fails with retarded sed

From: Mike Hommey <hidden>
Date: 2016-06-15 22:43:52

On Fri, Nov 16, 2007 at 06:58:21PM +0100, Benoit Sigoure wrote:
On Nov 16, 2007, at 6:26 PM, Mike Hommey wrote:
quoted
Brown paper bag fix to avoid test failure with retarded sed. The test
by itself didn't catch what it was supposed to, anyways.

So now, we test whether the editor gets at least an empty line, some
commented lines, and doesn't get anything else.

Signed-off-by: Mike Hommey <redacted>
---
 t/t7004-tag.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 096fe33..3813f23 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1007,7 +1007,9 @@ test_expect_failure \
 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
+	grep -e "^$" actual > /dev/null 2>&1 &&
+	grep -e "^#" actual > /dev/null 2>&1 &&
+	! grep -e "^[^#]" actual > /dev/null 2>&1
 '

 get_tag_header reuse $commit commit $time >expect
If your system has a retarded `sed', it will most likely also have a 
brain-dead `/bin/sh' which doesn't handle `! command'.  So I suggest you 
rewrite the last line as:
  grep -ve "^[^#]" actual > /dev/null 2>&1
A whole lot of the test suite uses `! command', which is why i chose to
use it.

Mike
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help