From: Victoria Dye <vdye@github.com>
Date: Fri, 27 Jan 2023 19:15:09 GMT
Subject: Re: [PATCH v2 02/10] t5558: add tests for creationToken heuristic
Message-ID: <39e4a13a-2230-919f-4cfc-95d70fb33e3a@github.com>
In-Reply-To: <427aff4d5e5c85b601f43af8b664515380e11453.1674487310.git.gitgitgadget@gmail.com>

Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <derrickstolee@github.com>
> 
> 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.