> Sean McAllister <smcallis@xxxxxxxxxx> writes:
>
> > +# generate a random 12 digit string
> > +gen_nonce() {
> > + test_copy_bytes 12 < /dev/urandom | tr -dc A-Za-z0-9
> > +}
>
> What is the randomness requirement of this application? IOW, what
> breaks if we just change this to "echo 0123456789ab"?
>
> Or "date | git hash-object --stdin" for that matter?
>
> We'd want to make our tests more predictiable, not less.
The randomness requirement is just that I need nonces to be unique
during a single run of the HTTP server
as they uniquefy the files I put on disk to make the HTTP hack-ily
stateful. I'd be fine with your date/hash-object
solution, but I don't know that it will help make the tests more predictable.
>
> > diff --git a/t/lib-httpd/error-ntime.sh b/t/lib-httpd/error-ntime.sh
> > new file mode 100755
> > index 0000000000..e4f91ab816
> > --- /dev/null
> > +++ b/t/lib-httpd/error-ntime.sh
> > @@ -0,0 +1,79 @@
> > +#!/bin/sh
> > +
> > +# Script to simulate a transient error code with Retry-After header set.
> > +#
> > +# PATH_INFO must be of the form /<nonce>/<times>/<retcode>/<retry-after>/<path>
> > +# (eg: /dc724af1/3/429/10/some/url)
> > +#
> > +# The <nonce> value uniquely identifies the URL, since we're simulating
> > +# a stateful operation using a stateless protocol, we need a way to "namespace"
> > +# URLs so that they don't step on each other.
> > +#
> > +# The first <times> times this endpoint is called, it will return the given
> > +# <retcode>, and if the <retry-after> is non-negative, it will set the
> > +# Retry-After head to that value.
> > +#
> > +# Subsequent calls will return a 302 redirect to <path>.
> > +#
> > +# Supported error codes are 429,502,503, and 504
> > +
> > +print_status() {
> > + if [ "$1" -eq "302" ]; then printf "Status: 302 Found\n"
> > + elif [ "$1" -eq "429" ]; then printf "Status: 429 Too Many Requests\n"
> > + elif [ "$1" -eq "502" ]; then printf "Status: 502 Bad Gateway\n"
> > + elif [ "$1" -eq "503" ]; then printf "Status: 503 Service Unavailable\n"
> > + elif [ "$1" -eq "504" ]; then printf "Status: 504 Gateway Timeout\n"
> > + else
> > + printf "Status: 500 Internal Server Error\n"
> > + fi
> > + printf "Content-Type: text/plain\n"
>
> Style????? (I won't repeat this comment for the rest of this script)
>
> I briefly wondered "oh, are t/lib-httpd/* scripts excempt from the
> coding guidelines?" but a quick look at them tells me that that is
> not the case.
>
I mistakenly thought the Makefile in t/ was linting these as well.
I've gone back through and fixed formatting issues and removed
non-posix constructs.
> > +}
> > +
> > +# read in path split into cmoponents
> > +IFS='/'
> > +tokens=($PATH_INFO)
> > +
> > +# break out code/retry parts of path
> > +nonce=${tokens[1]}
> > +times=${tokens[2]}
> > +code=${tokens[3]}
> > +retry=${tokens[4]}
>
> You said /bin/sh upfront. Don't use non-POSIX shell arrays.
>
> > +
> > +# get redirect path
> > +cnt=0
> > +path=""
> > +for ((ii=0; ii < ${#PATH_INFO}; ii++)); do
> > + if [ "${PATH_INFO:${ii}:1}" == "/" ]; then
> > + let cnt=${cnt}+1
> > + fi
> > + if [ "${cnt}" -eq 5 ]; then
> > + path="${PATH_INFO:${ii}}"
> > + break
> > + fi
> > +done
> > +
> > +# leave a cookie for this request/retry count
> > +state_file="request_${REMOTE_ADDR}_${nonce}_${times}_${code}_${retry}"
> > +
> > +if [ ! -f "$state_file" ]; then
> > + echo 0 > "$state_file"
> > +fi
> > +
> > +
> > +read cnt < "$state_file"
> > +if [ "$cnt" -lt "$times" ]; then
> > + let cnt=cnt+1
> > + echo "$cnt" > "$state_file"
> > +
> > + # return error
> > + print_status "$code"
> > + if [ "$retry" -ge "0" ]; then
> > + printf "Retry-After: %s\n" "$retry"
> > + fi
> > +else
> > + # redirect
> > + print_status 302
> > + printf "Location: %s?%s\n" "$path" "${QUERY_STRING}"
> > +fi
> > +
> > +echo
> > diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> > index 7df3c5373a..71d4307001 100755
> > --- a/t/t5601-clone.sh
> > +++ b/t/t5601-clone.sh
> > @@ -756,6 +756,15 @@ test_expect_success 'partial clone using HTTP' '
> > partial_clone "$HTTPD_DOCUMENT_ROOT_PATH/server" "$HTTPD_URL/smart/server"
> > '
> >
> > +test_expect_success 'partial clone using HTTP with redirect' '
> > + _NONCE=`gen_nonce` && export _NONCE &&
> > + curl "$HTTPD_URL/error_ntime/${_NONCE}/3/502/10/smart/server" > /dev/null &&
> > + curl "$HTTPD_URL/error_ntime/${_NONCE}/3/502/10/smart/server" > /dev/null &&
> > + curl "$HTTPD_URL/error_ntime/${_NONCE}/3/502/10/smart/server" > /dev/null &&
>
> These lines are not indented with HT?
>
> Don't redirect test output to /dev/null, which is done by test_expect_success
> for us. >/dev/null makes it less useful to run the test under "-v" option.
>
Fixed in v2.
> > + partial_clone "$HTTPD_DOCUMENT_ROOT_PATH/server" "$HTTPD_URL/error_ntime/${_NONCE}/3/502/10/smart/server"
> > +'
> > +
> > +
> > # DO NOT add non-httpd-specific tests here, because the last part of this
> > # test script is only executed when httpd is available and enabled.
