Re: [GSoC][PATCH] t: migrate helper/test-urlmatch-normalization to unit tests

From: Patrick Steinhardt <ps@pks.im>
Date: Jul 23, 2024, 14:00 UTC
Message-ID: <Zp-3e6VV5bl8dWvR@tanuki>
In-Reply-To: <20240628125632.45603-1-shyamthakkar001@gmail.com>

On Fri, Jun 28, 2024 at 06:26:24PM +0530, Ghanshyam Thakkar wrote:

Show 14 quoted lines

> +static void compare_normalized_urls(const char *url1, const char *url2,
> +				    size_t equal)
> +{
> +	char *url1_norm = url_normalize(url1, NULL);
> +	char *url2_norm = url_normalize(url2, NULL);
> +
> +	if (equal) {
> +		if (!check_str(url1_norm, url2_norm))
> +			test_msg("input url1: %s\n  input url2: %s", url1,
> +				 url2);
> +	} else if (!check_int(strcmp(url1_norm, url2_norm), !=, 0))
> +		test_msg(" url1_norm: %s\n   url2_norm: %s\n"
> +			 "  input url1: %s\n  input url2: %s",
> +			 url1_norm, url2_norm, url1, url2);

Nit: this is missing braces around the `else if` branch. If one of the
conditional bodies has braces, then all should have according to our
style guide.

Show 10 quoted lines

> +	free(url1_norm);
> +	free(url2_norm);
> +}
> +
> +static void check_normalized_url_from_file(const char *file, const char *expect)
> +{
> +	struct strbuf content = STRBUF_INIT, path = STRBUF_INIT;
> +
> +	strbuf_getcwd(&path);
> +	strbuf_strip_suffix(&path, "/unit-tests/bin"); /* because 'unit-tests-test-tool' is run from 'bin' directory */

Curious: is this a new requirement or do other tests have the same
requirement? I was under the impression that I could execude the
resulting unit test binaries from whatever directory I wanted to, but
didn't verify.

In any case, the line should probably be wrapped as it is overly long.

Other than that this looks good to me. I've gave a cursory read to the testcases themselves and they do look like a faithful conversion to me.

Thanks!

Patrick