git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jeff Hostetler <git@jeffhostetler.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Jeff Hostetler <jeffhost@microsoft.com>,
	git@vger.kernel.org,
	"Randall S . Becker" <randall.becker@nexbridge.ca>
Subject: Re: [PATCH] trace2 tests: guard pthread test with "PTHREAD"
Date: Mon, 28 Nov 2022 18:50:19 +0100	[thread overview]
Message-ID: <221128.86wn7fj72k.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <6c725ed6-4d74-c260-98fb-dc965b4647ee@jeffhostetler.com>


On Mon, Nov 28 2022, Jeff Hostetler wrote:

> On 11/25/22 3:05 AM, Ævar Arnfjörð Bjarmason wrote:
> [...]
>> That was my thought as well, but these tests are specifically testing
>> how it interacts with threading. The counter mechanism works in general
>> without threading.
>> The test descriptions don't help, and should really say that they're
>> to
>> do with threading in particular, but I wanted to keep this as small as
>> possible for rc[12] and the final, so I didn't fix that while-at-it.
>
> There is large comment block above `have_timer_event()` and
> `have_counter_event()` in t0211 that explained the purpose of the
> test1 and test2 tests for each.  Would it help if that text were moved
> down before each of the individual tests rather than where it is now?

You did ask :)

I think that better than a comment is to have the test description
itself reflect the nature & purpose of the test.

Now the two are:

	test_expect_success 'global counter test/test1' '
	test_expect_success PTHREAD 'global counter test/test2' '

So at least the PTHREAD shows that it's something to do with threading,
but if it fails with that prereq passed you'll need to consult the
source to see what "test2" is supposed to do. Better would be to just
skip the comment & work "single threaded" and "multi-threaded" etc. into
the test name itself.

Ditto symbol names "ut_200counter" and "ut_201counter", again, a comment
somewhere in t/helper/test-trace2.c notes that they're single- and
multi-threaded, respectively, but why not skip that and make the symbol
names self-descriptive?

  reply	other threads:[~2022-11-28 18:15 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <010201d9002ee2f9940nexbridge.com>
2022-11-24 21:48 ` [PATCH] trace2 tests: guard pthread test with "PTHREAD" Ævar Arnfjörð Bjarmason
2022-11-25  6:41   ` Junio C Hamano
2022-11-25  8:05     ` Ævar Arnfjörð Bjarmason
2022-11-28 16:37       ` Jeff Hostetler
2022-11-28 17:50         ` Ævar Arnfjörð Bjarmason [this message]
2022-11-30 18:41           ` Jeff Hostetler

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=221128.86wn7fj72k.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=randall.becker@nexbridge.ca \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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).