Re: [PATCH] remote-curl: add testing for intelligent retry for HTTP

Sean McAllister [off-list ref] writes:

quoted

+# 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.

quoted

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.

quoted

+}
+
+# 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.

quoted

+
+# 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.

quoted

+     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.