On Fri, Jun 28, 2024 at 06:26:24PM +0530, Ghanshyam Thakkar wrote:
> +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.
> + 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
Attachment:
signature.asc
Description: PGP signature
