git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Cc: git@vger.kernel.org,  Patrick Steinhardt <ps@pks.im>,
	 Karthik Nayak <karthik.188@gmail.com>,
	 Phillip Wood <phillip.wood123@gmail.com>,
	Christian Couder <christian.couder@gmail.com>,
	 Christian Couder <chriscool@tuxfamily.org>,
	 Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Subject: Re: [GSoC][PATCH v2] t: migrate t0110-urlmatch-normalization to the new framework
Date: Tue, 13 Aug 2024 12:22:52 -0700	[thread overview]
Message-ID: <xmqqh6bo448j.fsf@gitster.g> (raw)
In-Reply-To: <20240813172432.55487-1-shyamthakkar001@gmail.com> (Ghanshyam Thakkar's message of "Tue, 13 Aug 2024 22:54:21 +0530")

Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:

> With the addition of this unit test, we impose a new restriction of
> running the unit tests from either 't/' or 't/unit-tests/bin/'
> directories. This is to construct the path to files which contain some
> input urls under the 't/t-urlmatch-normalization' directory. This
> restriction is similar to one we have for end-to-end tests, where they
> can be ran from only 't/'.
>
> Addition of 't/unit-tests/bin/' is to allow
> for running individual tests which is not currently possible via any
> 'make' targets and also 'unit-tests-test-tool' target is also ran from
> the 't/unit-tests/bin' directory.

Sorry, but I do not quite follow.  The above makes it sound as if
the 'bin' subdirectory is something that never existed before this
patch and this patch introduces the use of that directory, but that
is hardly the case.  What does that "Addition of" really refer to?

Do you mean "we cannot run the tests from arbitrary places, and we
allow them to be run from t/, just like the normal tests" followed
by "in addition, we also allow them to be run from t/unit-tests/bin
directory because ..."?

I wonder if we should get of t/t-urlmatch-normalization/ directory
and instead hold these test data in the form of string constants in
the program.  After all, you have the expected normalization result
hardcoded in the binary (e.g. t_url_high_bit() asks the checker
function to read from "url-1" file and then compare the result of
normalization with a hardcoded string constant), so having the test
data in separate files only risks the input and the output easily
drift apart.

As a side effect, it would make it easily possible to run the tests
anywhere, because you no longer depend on these url-$n input files.
It of course depends on how burdensome the limitation that we can
run the tests only from a fixed place really is, but it generally is
not a good idea to have these random sequence of bytes in small
files that nobody looks at in a repository in the first place.

Thanks.

  reply	other threads:[~2024-08-13 19:22 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-28 12:56 [GSoC][PATCH] t: migrate helper/test-urlmatch-normalization to unit tests Ghanshyam Thakkar
2024-07-09  0:42 ` Ghanshyam Thakkar
2024-07-22 12:53 ` Karthik Nayak
2024-07-22 12:54   ` Ghanshyam Thakkar
2024-07-23  8:26     ` Karthik Nayak
2024-07-23 14:00 ` Patrick Steinhardt
2024-07-24  0:24   ` Ghanshyam Thakkar
2024-07-24  5:19     ` Patrick Steinhardt
2024-07-24  7:06       ` Ghanshyam Thakkar
2024-07-24  7:45         ` Patrick Steinhardt
2024-08-13 17:24 ` [GSoC][PATCH v2] t: migrate t0110-urlmatch-normalization to the new framework Ghanshyam Thakkar
2024-08-13 19:22   ` Junio C Hamano [this message]
2024-08-14  1:35     ` Kaartic Sivaraam
2024-08-14  4:58       ` Junio C Hamano
2024-08-14 14:24     ` Ghanshyam Thakkar
2024-08-14  5:17   ` Kaartic Sivaraam
2024-08-14 14:20   ` [GSoC][PATCH v3] " Ghanshyam Thakkar
2024-08-14 16:52     ` Junio C Hamano
2024-08-19 12:46     ` Christian Couder
2024-08-20 15:19     ` [GSoC][PATCH v4] " Ghanshyam Thakkar
2024-08-20 15:24       ` Ghanshyam Thakkar
2024-08-21 10:06       ` Christian Couder
2024-08-21 16:08         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqh6bo448j.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=kaartic.sivaraam@gmail.com \
    --cc=karthik.188@gmail.com \
    --cc=phillip.wood123@gmail.com \
    --cc=ps@pks.im \
    --cc=shyamthakkar001@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).