Subject: Re: [PATCH v2 02/10] t5558: add tests for creationToken heuristic

From: Victoria Dye <[email-masked]>
Date: Fri, 27 Jan 2023 11:15:09 -0800

Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <[email-masked]>
> 
> As documented in the bundle URI design doc in 2da14fad8fe (docs:
> document bundle URI standard, 2022-08-09), the 'creationToken' member of
> a bundle URI allows a bundle provider to specify a total order on the
> bundles.
> 
> Future changes will allow the Git client to understand these members and
> modify its behavior around downloading the bundles in that order. In the
> meantime, create tests that add creation tokens to the bundle list. For
> now, the Git client correctly ignores these unknown keys.
> 
> Create a new test helper function, test_remote_https_urls, which filters
> GIT_TRACE2_EVENT output to extract a list of URLs passed to
> git-remote-https child processes. This can be used to verify the order
> of these requests as we implement the creationToken heuristic. For now,
> we need to sort the actual output since the current client does not have
> a well-defined order that it applies to the bundles.

...

> +# Given a GIT_TRACE2_EVENT log over stdin, writes to stdout a list of URLs
> +# sent to git-remote-https child processes.
> +test_remote_https_urls() {
> +	grep -e '"event":"child_start".*"argv":\["git-remote-https",".*"\]' |
> +		sed -e 's/{"event":"child_start".*"argv":\["git-remote-https","//g' \
> +		    -e 's/"\]}//g'
> +}
> +

...

> +	cat >expect <<-EOF &&
> +	$HTTPD_URL/bundle-1.bundle
> +	$HTTPD_URL/bundle-2.bundle
> +	$HTTPD_URL/bundle-3.bundle
> +	$HTTPD_URL/bundle-4.bundle
> +	$HTTPD_URL/bundle-list
> +	EOF
> +
> +	# Sort the list, since the order is not well-defined
> +	# without a heuristic.
> +	test_remote_https_urls <trace-clone.txt | sort >actual &&
> +	test_cmp expect actual

...

> +	cat >expect <<-EOF &&
> +	$HTTPD_URL/bundle-1.bundle
> +	$HTTPD_URL/bundle-2.bundle
> +	$HTTPD_URL/bundle-3.bundle
> +	$HTTPD_URL/bundle-4.bundle
> +	$HTTPD_URL/bundle-list
> +	EOF
> +
> +	# Since the creationToken heuristic is not yet understood by the
> +	# client, the order cannot be verified at this moment. Sort the
> +	# list for consistent results.
> +	test_remote_https_urls <trace-clone.txt | sort >actual &&
> +	test_cmp expect actual

These updates make the tests stronger (that is, less likely to let a
regression slip through), and the additional comments are helpful for
explaining what is and is not implemented at this point in the series.