Re: CVS commit: src/tests/kernel

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

> Module Name:    src
> Committed By:   yamt
> Date:           Tue Mar 17 08:10:58 UTC 2026
> 
> Modified Files:
>         src/tests/kernel: t_time_arith.c
> 
> Log Message:
> tests/kernel/t_time_arith.c: remove tests which don't make much sense
> 
> remove tests which use ms and us values as ns because:
> 
> * they don't make much sense.
> 
> * their expected results assume a particular implementation.
> 
> @@ -242,58 +242,6 @@ const struct itimer_transition {
>  		{2,999999999}, {0,0}, 0,
>  		NULL},
>  
> -	/* (2^63 - 1) us */
> -	[32] = {{.it_value = {3,0}, .it_interval = {9223372036854,775807}},
> -		{2,999999999}, {0,0}, 0,
> -		NULL},
> -	/* 2^63 us */
> ...

The reasoning behind these test cases is:

1. It is critically important to make sure the code gives _some_
   sensible answer instead of undefined behaviour of arithmetic
   overflow for all inputs.

   We have had a spate of bugs in that area, and I spent a lot of
   effort squashing them.  That entailed coming up with and thinking
   about sensible results of all of these test cases -- I didn't
   simply call the function as currently implemented and record its
   outputs; I reasoned through what I expected for every case, and
   then adjusted as appropriate when my reasoning was wrong.

2. Edge cases are important to test.  Although these aren't really
   edge cases of _this_ implementation since the implementation
   currently works in nanoseconds, they _would_ be edge cases if the
   implementation were changed to work in microseconds or
   milliseconds (which would, in turn, be reasonable given our low
   resolution of sleep times anyway).

3. The answers may not be technically correct in theory, but they are
   applicable only to time intervals that are so large -- 2^63 us is
   nearly three hundred millennia -- that it is reasonable for us to
   treat periodic intervals of that duration as `never' in practice.

So please put these test cases back.

If you want to mark them as with a comment explaining that they're not
technically correct, that's fine.  If you want to implement
timespecdiv and make the answers more technically correct, that's fine
too.  But please don't just drop tests that exercise these plausible
edge cases.

> @@ -406,6 +354,9 @@ ATF_TC_BODY(itimer_transitions, tc)
>  		volatile bool aborted = true;
>  		volatile bool expect_abort = false;
>  
> +		if (!timespecisset(&it.it_time.it_interval))
> +			continue;
> +
>  		fprintf(stderr, "case %u\n", i);

I guess this doesn't hurt since (a) none of the explicitly written
test cases use a zero interval, and (b) the kern_time.c caller only
uses itimer_transition if the interval is nonzero anyway.  But it
would be nice to have a comment here explaining this; I spent a few
minutes puzzling over this part of the change before I realized it.