Message ID | 20190208220751.9936-1-randall.s.becker@rogers.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [Fix,v2] t5562: remove dependency on /dev/zero | expand |
On Fri, Feb 08, 2019 at 05:07:51PM -0500, randall.s.becker@rogers.com wrote: > From: "Randall S. Becker" <rsbecker@nexbridge.com> > > Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero > with yes and a translation of its result to a stream of NULL. This is > a more portable solution. They're NULs, not NULLs. :) I think in this case, though, we don't actually care about the bytes, and you can drop the "tr" invocation entirely. -Peff
randall.s.becker@rogers.com writes: > From: "Randall S. Becker" <rsbecker@nexbridge.com> > > Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero > with yes and a translation of its result to a stream of NULL. This is > a more portable solution. > > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> > --- > t/t5562-http-backend-content-length.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh > index 90d890d02..b8d1913e5 100755 > --- a/t/t5562-http-backend-content-length.sh > +++ b/t/t5562-http-backend-content-length.sh > @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' ' > > test_expect_success 'CONTENT_LENGTH overflow ssite_t' ' > NOT_FIT_IN_SSIZE=$(ssize_b100dots) && > - env \ > + yes | tr "y" "\\0" | env \ I do not quite get this use of tr. The original feeds a stream of NULs out of /dev/zero to the command; the yes-to-tr pipe instead feeds a stream of alternating NUL and LF. Does the actual bytes fed to the consumer make any difference? If not, perhaps we can use 'yes' as-is? > CONTENT_TYPE=application/x-git-upload-pack-request \ > QUERY_STRING=/repo.git/git-upload-pack \ > PATH_TRANSLATED="$PWD"/.git/git-upload-pack \ > GIT_HTTP_EXPORT_ALL=TRUE \ > REQUEST_METHOD=POST \ > CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \ > - git http-backend </dev/zero >/dev/null 2>err && > + git http-backend >/dev/null 2>err && > grep "fatal:.*CONTENT_LENGTH" err > '
Am 08.02.19 um 23:07 schrieb randall.s.becker@rogers.com: > From: "Randall S. Becker" <rsbecker@nexbridge.com> > > Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero > with yes and a translation of its result to a stream of NULL. This is > a more portable solution. > > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> > --- > t/t5562-http-backend-content-length.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh > index 90d890d02..b8d1913e5 100755 > --- a/t/t5562-http-backend-content-length.sh > +++ b/t/t5562-http-backend-content-length.sh > @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' ' > > test_expect_success 'CONTENT_LENGTH overflow ssite_t' ' > NOT_FIT_IN_SSIZE=$(ssize_b100dots) && > - env \ > + yes | tr "y" "\\0" | env \ > CONTENT_TYPE=application/x-git-upload-pack-request \ > QUERY_STRING=/repo.git/git-upload-pack \ > PATH_TRANSLATED="$PWD"/.git/git-upload-pack \ > GIT_HTTP_EXPORT_ALL=TRUE \ > REQUEST_METHOD=POST \ > CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \ > - git http-backend </dev/zero >/dev/null 2>err && > + git http-backend >/dev/null 2>err && > grep "fatal:.*CONTENT_LENGTH" err > ' How many bytes are needed here? yes() in test-lib.sh generates only 99 'y', if I am not mistaken. -- Hannes
On February 8, 2019 18:39, Junio C Hamano wrote: > randall.s.becker@rogers.com writes: > > Replaced subtest 15 (CONTENT_LENGTH overflow ssite_t) use of /dev/zero > > with yes and a translation of its result to a stream of NULL. This is > > a more portable solution. > > > > Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com> > > --- > > t/t5562-http-backend-content-length.sh | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/t/t5562-http-backend-content-length.sh > > b/t/t5562-http-backend-content-length.sh > > index 90d890d02..b8d1913e5 100755 > > --- a/t/t5562-http-backend-content-length.sh > > +++ b/t/t5562-http-backend-content-length.sh > > @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' ' > > > > test_expect_success 'CONTENT_LENGTH overflow ssite_t' ' > > NOT_FIT_IN_SSIZE=$(ssize_b100dots) && > > - env \ > > + yes | tr "y" "\\0" | env \ > > I do not quite get this use of tr. The original feeds a stream of NULs out of > /dev/zero to the command; the yes-to-tr pipe instead feeds a stream of > alternating NUL and LF. That's why we're going to go with a generate_zero_bytes function per Peff. I'm working on a more comprehensive patch covering t5562, t5318, and test-lib-functions.sh that will (hopefully) be satisfactory and remove the dependency on /dev/zero and fixes the related new breakages in 2.21.0-rc0. The test case in t5318 is specific about wanting zero bytes although the test is just intending to generate a corrupt block that generates a different hash, so yes 'yes' might be sufficient, but I don't like randomness myself if we're taking two different tests being involved. My current intent is to add to test-lib-functions.sh, a method of generalizing blocks of zeros to a pipe: +# Generate an output of $1 bytes of all zeroes (NULs, not ASCII zeroes). +# If $1 is < 0, output forever or until the receiving pipe stops reading, whichever comes first. + generate_zero_bytes () { + perl -e ' if ($ARGV[0] < 0) { while (-1) { print "\0" } } else { print "\0" x $ARGV[0] }' "$@" + } And then fit that into the two tests, then submit as a patch. > Does the actual bytes fed to the consumer make any difference? If not, > perhaps we can use 'yes' as-is? > > > CONTENT_TYPE=application/x-git-upload-pack-request \ > > QUERY_STRING=/repo.git/git-upload-pack \ > > PATH_TRANSLATED="$PWD"/.git/git-upload-pack \ > > GIT_HTTP_EXPORT_ALL=TRUE \ > > REQUEST_METHOD=POST \ > > CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \ > > - git http-backend </dev/zero >/dev/null 2>err && > > + git http-backend >/dev/null 2>err && > > grep "fatal:.*CONTENT_LENGTH" err > > ' Regards, Randall
Johannes Sixt <j6t@kdbg.org> writes: > How many bytes are needed here? yes() in test-lib.sh generates only 99 > 'y', if I am not mistaken. I think we will not use "yes" in the end for this topic, which makes this comment totally irrelevant to the thread, but I wonder why we have the limit of 99 there? It cannot be "we do not want to worry about sigpipe" affecting the end result of the test (after all the reader may stop reading from after reading just one, and the status of the upstream process that would die with sigpipe is lost anyway). It turns out it is about sigpipe ;-) but in somewhat a different way. To prevent others from wasting their time wondering about this, probably we want to have something like the attached? t/README | 9 +++++++++ t/test-lib.sh | 6 +++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/t/README b/t/README index 1326fd7505..f4e1a82657 100644 --- a/t/README +++ b/t/README @@ -927,6 +927,15 @@ library for your script to use. test_oid_init or test_oid_cache. Providing an unknown key is an error. + - yes [<string>] + + This is often seen in modern UNIX but some platforms lack it, so + the test harness overrides the platform implementation with a + more limited one. Use this only when feeding a handful lines of + output to the downstream---unlike the real version, it generates + only up to 99 lines. + + Prerequisites ------------- diff --git a/t/test-lib.sh b/t/test-lib.sh index 42b1a0aa7f..541a37f4c0 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1313,7 +1313,11 @@ then fi fi -# Provide an implementation of the 'yes' utility +# Provide an implementation of the 'yes' utility; the upper bound +# limit is there to help Windows that cannot stop this loop from +# wasting cycles when the downstream stops reading, so do not be +# tempted to turn it into an infinite loop. cf. 6129c930 ("test-lib: +# limit the output of the yes utility", 2016-02-02) yes () { if test $# = 0 then
On February 9, 2019 13:25, Junio C Hamano wrote: > Johannes Sixt <j6t@kdbg.org> writes: > > > How many bytes are needed here? yes() in test-lib.sh generates only 99 > > 'y', if I am not mistaken. > > I think we will not use "yes" in the end for this topic, which makes this > comment totally irrelevant to the thread, but I wonder why we have the limit > of 99 there? It cannot be "we do not want to worry about sigpipe" affecting > the end result of the test (after all the reader may stop reading from after > reading just one, and the status of the upstream process that would die with > sigpipe is lost anyway). > > It turns out it is about sigpipe ;-) but in somewhat a different way. To > prevent others from wasting their time wondering about this, probably we > want to have something like the attached? > > t/README | 9 +++++++++ > t/test-lib.sh | 6 +++++- > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/t/README b/t/README > index 1326fd7505..f4e1a82657 100644 > --- a/t/README > +++ b/t/README > @@ -927,6 +927,15 @@ library for your script to use. > test_oid_init or test_oid_cache. Providing an unknown key is an > error. > > + - yes [<string>] > + > + This is often seen in modern UNIX but some platforms lack it, so > + the test harness overrides the platform implementation with a > + more limited one. Use this only when feeding a handful lines of > + output to the downstream---unlike the real version, it generates > + only up to 99 lines. > + > + > Prerequisites > ------------- > > diff --git a/t/test-lib.sh b/t/test-lib.sh index 42b1a0aa7f..541a37f4c0 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -1313,7 +1313,11 @@ then > fi > fi > > -# Provide an implementation of the 'yes' utility > +# Provide an implementation of the 'yes' utility; the upper bound # > +limit is there to help Windows that cannot stop this loop from # > +wasting cycles when the downstream stops reading, so do not be # > +tempted to turn it into an infinite loop. cf. 6129c930 ("test-lib: > +# limit the output of the yes utility", 2016-02-02) > yes () { > if test $# = 0 > then Sadly, I already the other path ready, but did not have a chance to send it. I'm ok either way as long as I can get the tests running. Regards, Randall
Am 09.02.19 um 19:25 schrieb Junio C Hamano: > Johannes Sixt <j6t@kdbg.org> writes: > >> How many bytes are needed here? yes() in test-lib.sh generates only 99 >> 'y', if I am not mistaken. > > I think we will not use "yes" in the end for this topic, which makes > this comment totally irrelevant to the thread, but I wonder why we > have the limit of 99 there? It cannot be "we do not want to worry > about sigpipe" affecting the end result of the test (after all the > reader may stop reading from after reading just one, and the status > of the upstream process that would die with sigpipe is lost anyway). > > It turns out it is about sigpipe ;-) but in somewhat a different > way. To prevent others from wasting their time wondering about > this, probably we want to have something like the attached? That would certainly be helpful! -- Hannes > > t/README | 9 +++++++++ > t/test-lib.sh | 6 +++++- > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/t/README b/t/README > index 1326fd7505..f4e1a82657 100644 > --- a/t/README > +++ b/t/README > @@ -927,6 +927,15 @@ library for your script to use. > test_oid_init or test_oid_cache. Providing an unknown key is an > error. > > + - yes [<string>] > + > + This is often seen in modern UNIX but some platforms lack it, so > + the test harness overrides the platform implementation with a > + more limited one. Use this only when feeding a handful lines of > + output to the downstream---unlike the real version, it generates > + only up to 99 lines. > + > + > Prerequisites > ------------- > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 42b1a0aa7f..541a37f4c0 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -1313,7 +1313,11 @@ then > fi > fi > > -# Provide an implementation of the 'yes' utility > +# Provide an implementation of the 'yes' utility; the upper bound > +# limit is there to help Windows that cannot stop this loop from > +# wasting cycles when the downstream stops reading, so do not be > +# tempted to turn it into an infinite loop. cf. 6129c930 ("test-lib: > +# limit the output of the yes utility", 2016-02-02) > yes () { > if test $# = 0 > then >
diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh index 90d890d02..b8d1913e5 100755 --- a/t/t5562-http-backend-content-length.sh +++ b/t/t5562-http-backend-content-length.sh @@ -143,14 +143,14 @@ test_expect_success GZIP 'push gzipped empty' ' test_expect_success 'CONTENT_LENGTH overflow ssite_t' ' NOT_FIT_IN_SSIZE=$(ssize_b100dots) && - env \ + yes | tr "y" "\\0" | env \ CONTENT_TYPE=application/x-git-upload-pack-request \ QUERY_STRING=/repo.git/git-upload-pack \ PATH_TRANSLATED="$PWD"/.git/git-upload-pack \ GIT_HTTP_EXPORT_ALL=TRUE \ REQUEST_METHOD=POST \ CONTENT_LENGTH="$NOT_FIT_IN_SSIZE" \ - git http-backend </dev/zero >/dev/null 2>err && + git http-backend >/dev/null 2>err && grep "fatal:.*CONTENT_LENGTH" err '