From: Glen Choo <chooglen@google.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Kousik Sanagavarapu <five231003@gmail.com>,
git@vger.kernel.org,
Christian Couder <christian.couder@gmail.com>,
Hariom Verma <hariom18599@gmail.com>,
Josh Steadmon <steadmon@google.com>,
Siddharth Singh <siddhartth@google.com>
Subject: Re: [PATCH v3 1/2] ref-filter: add multiple-option parsing functions
Date: Fri, 21 Jul 2023 11:26:38 -0700 [thread overview]
Message-ID: <kl6lr0p1yvc1.fsf@chooglen-macbookpro.roam.corp.google.com> (raw)
In-Reply-To: <xmqqzg3q1g2y.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Glen Choo <chooglen@google.com> writes:
>
>>> So whatever framework we choose, it should allow adding a
>>> test or two to this patch easily, without being too
>>> intrusive. Would that be a good and concrete evaluation
>>> criterion?
>>
>> Perhaps, but the biggest blocker to adding a unit tests is whether the
>> source file itself is amenable to being unit tested (e.g. does it depend
>> on global state? does it compile easily?).
>
> Now would this particular example, the change to ref-filter.c file,
> be a reasonable guinea-pig test case for candidate test frameworks
> to add tests for these two helper functions? They are pretty-much
> plain vanilla string manipulation functions that does not depend too
> many things that are specific to Git.
Ah, yes. This would be close-to-ideal candidate then.
> They may use helpers we
> wrote, i.e. xstrndup(), skip_prefix(), and git_parse_maybe_bool(),
> but they shouldn't depend on the program start-up sequence,
> discovering repositories, installing at-exit handers, and other
> stuff. It was why I wondered if it can be used as a good evaluation
> criterion---if a test framework cannot easily add tests while this
> patch was being proposed in a non-intrusive way to demonstrate how
> these two functions are supposed to work and to protect their
> implementations from future breakage, it would not be all that
> useful, I would imagine.
The current thinking among Googlers is that we won't remove the helpers
from library code. Git will either provide them, e.g. via Calvin's
git-std-lib RFC [1], or we will provide ways for callers to bring their
own implementation (like trace2 or exit, since it doesn't necessarily
make sense to use Git's implementation). So yes, the test framework
should be able to support this sort of compilation pattern. I'm not sure
how much test frameworks differ in this regard, maybe Josh has some
insight here.
[1] https://lore.kernel.org/git/20230627195251.1973421-1-calvinwan@google.com
next prev parent reply other threads:[~2023-07-21 18:26 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-05 17:57 [PATCH 0/2] Add new "describe" atom Kousik Sanagavarapu
2023-07-05 17:57 ` [PATCH 1/2] ref-filter: add " Kousik Sanagavarapu
2023-07-06 16:58 ` Junio C Hamano
2023-07-09 6:16 ` Kousik Sanagavarapu
2023-07-05 17:57 ` [PATCH 2/2] t6300: run describe atom tests on a different repo Kousik Sanagavarapu
2023-07-14 19:20 ` [PATCH v2 0/3] Add new "describe" atom Kousik Sanagavarapu
2023-07-14 19:20 ` [RFC PATCH v2 1/3] ref filter: add multiple-option parsing functions Kousik Sanagavarapu
2023-07-14 19:20 ` [PATCH v2 2/3] ref-filter: add new "describe" atom Kousik Sanagavarapu
2023-07-14 20:57 ` Junio C Hamano
2023-07-15 18:24 ` Kousik Sanagavarapu
2023-07-15 18:56 ` Junio C Hamano
2023-07-14 19:20 ` [PATCH v2 3/3] t6300: run describe atom tests on a different repo Kousik Sanagavarapu
2023-07-19 16:15 ` [PATCH v3 0/2] Add new "describe" atom Kousik Sanagavarapu
2023-07-19 16:15 ` [PATCH v3 1/2] ref-filter: add multiple-option parsing functions Kousik Sanagavarapu
2023-07-19 23:23 ` Junio C Hamano
2023-07-20 5:21 ` Junio C Hamano
2023-07-20 16:52 ` Kousik Sanagavarapu
2023-07-20 17:59 ` Junio C Hamano
2023-07-20 17:42 ` Glen Choo
2023-07-20 20:30 ` Junio C Hamano
2023-07-21 18:26 ` Glen Choo [this message]
2023-07-19 16:15 ` [PATCH v3 2/2] ref-filter: add new "describe" atom Kousik Sanagavarapu
2023-07-19 22:56 ` Junio C Hamano
2023-07-20 22:52 ` [PATCH v3 0/2] Add " Junio C Hamano
2023-07-20 23:10 ` Junio C Hamano
2023-07-21 4:17 ` Kousik Sanagavarapu
2023-07-23 16:19 ` [PATCH v4 " Kousik Sanagavarapu
2023-07-23 16:19 ` [PATCH v4 1/2] ref-filter: add multiple-option parsing functions Kousik Sanagavarapu
2023-07-24 17:29 ` Junio C Hamano
2023-07-24 18:12 ` Kousik Sanagavarapu
2023-07-24 20:39 ` Junio C Hamano
2023-07-25 19:27 ` Junio C Hamano
2023-07-23 16:19 ` [PATCH v4 2/2] ref-filter: add new "describe" atom Kousik Sanagavarapu
2023-07-24 17:21 ` Junio C Hamano
2023-07-25 20:51 ` [PATCH v5 0/2] Add " Kousik Sanagavarapu
2023-07-25 20:51 ` [PATCH v5 1/2] ref-filter: add multiple-option parsing functions Kousik Sanagavarapu
2023-07-25 20:51 ` [PATCH v5 2/2] ref-filter: add new "describe" atom Kousik Sanagavarapu
2023-07-25 21:46 ` [PATCH v5 0/2] Add " 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=kl6lr0p1yvc1.fsf@chooglen-macbookpro.roam.corp.google.com \
--to=chooglen@google.com \
--cc=christian.couder@gmail.com \
--cc=five231003@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=hariom18599@gmail.com \
--cc=siddhartth@google.com \
--cc=steadmon@google.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 read-only IMAP folder(s) and NNTP newsgroup(s).