On 11/28/22 12:50 PM, Ævar Arnfjörð Bjarmason wrote:
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 :)
Yes, I did. :-) :-)
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?
Good points. I'll keep this in mind for the future. I don't
think I want to send a fix for them now, but maybe the next
time I'm in the area...
Thanks again,
Jeff
