Messages in this thread | | | From | Rae Moar <> | Date | Thu, 14 Mar 2024 15:44:23 -0400 | Subject | Re: [PATCH] kunit: time: Add faster unit test with shorter time range |
| |
On Thu, Mar 14, 2024 at 3:30 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On 3/14/24 12:05, Rae Moar wrote: > > On Wed, Mar 13, 2024 at 5:01 PM Guenter Roeck <linux@roeck-us.net> wrote: > >> > >> Commit a547c4ce10bd ("kunit: time: Mark test as slow using test > >> attributes") marked the time unit test as slow. This means it does not > >> run anymore if slow tests are disabled. This reduces test coverage and > >> is thus undesirable. At the same time, the test currently covers a range > >> of 160,000 years, which has limited value. > >> > >> Add additional test case covering a total range of 1,600 years. This test > >> takes less than a second to run even on slow systems while still covering > >> twice the leap year calculation range of 400 years around the center date. > >> This test can run even with slow tests disabled. > > > > Hello! > > > > I really like this addition of another time range test. This looks good to me. > > > > Thanks! > > -Rae > > > > Reviewed-by: Rae Moar <rmoar@google.com> > > > >> > >> Cc: Rae Moar <rmoar@google.com> > >> Cc: Shuah Khan <skhan@linuxfoundation.org> > >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> > >> --- > >> kernel/time/time_test.c | 35 +++++++++++++++++++++++++++-------- > >> 1 file changed, 27 insertions(+), 8 deletions(-) > >> > >> diff --git a/kernel/time/time_test.c b/kernel/time/time_test.c > >> index 3e5d422dd15c..15c6f3a5e73c 100644 > >> --- a/kernel/time/time_test.c > >> +++ b/kernel/time/time_test.c > >> @@ -47,18 +47,18 @@ static void advance_date(long *year, int *month, int *mday, int *yday) > >> } > >> > >> /* > >> - * Checks every day in a 160000 years interval centered at 1970-01-01 > >> + * Checks every day in a specified interval centered at 1970-01-01 > >> * against the expected result. > >> */ > >> -static void time64_to_tm_test_date_range(struct kunit *test) > >> +static void time64_to_tm_test_date_range(struct kunit *test, int years) > >> { > >> /* > >> - * 80000 years = (80000 / 400) * 400 years > >> - * = (80000 / 400) * 146097 days > >> - * = (80000 / 400) * 146097 * 86400 seconds > >> + * years = (years / 400) * 400 years > > > > This is tiny but if there is another version, I find this comment a > > bit confusing. Could you change this to maybe be "total seconds =" > > instead of "years =" because years is used as a unit on the right side > > of the equation? > > > > Good point. "total seconds" might be just as confusing, though. > How about "total range", "time range", or just "range" ? >
I see that. Any of those options look fine to me, maybe just "range"? Whatever you think is best of those.
Thanks! -Rae
> Thanks, > Guenter >
| |