Re: [GSoC][PATCH v2] t: migrate t0110-urlmatch-normalization to the new framework

From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Date: Aug 14, 2024, 05:17 UTC
Message-ID: <4c14e3bf-0f0d-4600-8e17-5b5d15ad7447@gmail.com>
In-Reply-To: <20240813172432.55487-1-shyamthakkar001@gmail.com>

Hi Ghansyam,

I just wanted to share two comments based on what I observed from the recent changes.

On 13/08/24 22:54, Ghanshyam Thakkar wrote:
 >

Show 15 quoted lines

> index 0000000000..e0dd50dc11
> --- /dev/null
> +++ b/t/unit-tests/t-urlmatch-normalization.c
> @@ -0,0 +1,294 @@
> +
> +static void check_normalized_url_from_file(const char *file, const char *expect)
> +{
> +	struct strbuf content = STRBUF_INIT, path = STRBUF_INIT;
> +	char *cwd_basename;
> +
> +	if (!check_int(strbuf_getcwd(&path), ==, 0))
> +		return;
> +
> +	cwd_basename = basename(path.buf);
> +	if (!check(!strcmp(cwd_basename, "t") || !strcmp(cwd_basename, "bin"))) {

I think comparing blindly for "bin" would not be an ideal thing to do as this would let other locations which have the "bin" basename to be slip through. For instance, running the test from "perl/blib/bin" would result in this check letting it through. I suppose we could need a bit more specific check that ensures that the test is indeed running from "t/unit-tests/bin".

> ... snip ...

Show 6 quoted lines

> +static void t_url_high_bit(void)
> +{
> +	check_normalized_url_from_file("url-1",
> +			    "x://q/%01%02%03%04%05%06%07%08%0E%0F%10%11%12");
> +	check_normalized_url_from_file("url-2",
> +			    "x://q/%13%14%15%16%17%18%19%1B%1C%1D%1E%1F%7F");

When we run the unit-test binary from other directories, the error message is thrown appropriately. But it seems to be printed for every test case which seems a bit spammy. I suppose it might be helpful to do it once before actually running the test cases and skip the cases when we realize the binary is being run from a different directory.

-- Sivaraam