* [PATCH] tst-getdate: Improve testcase flexibility and add test.
@ 2023-06-09 15:59 Joe Simmons-Talbott
2023-06-09 17:20 ` Arjun Shankar
0 siblings, 1 reply; 3+ messages in thread
From: Joe Simmons-Talbott @ 2023-06-09 15:59 UTC (permalink / raw)
To: libc-alpha; +Cc: Joe Simmons-Talbott
The getdate testcases all expect successful results. Add support for
negative testcases and testcases where a full date and time are not
supplied by skipping the tm checks in the test. Add a testcase that
would catch a use-after-free that was recently found.
---
time/tst-getdate.c | 56 ++++++++++++++++++++++++++++++----------------
1 file changed, 37 insertions(+), 19 deletions(-)
diff --git a/time/tst-getdate.c b/time/tst-getdate.c
index 4c9ed28d58..0036d313d7 100644
--- a/time/tst-getdate.c
+++ b/time/tst-getdate.c
@@ -32,34 +32,40 @@ static const struct
const char *tz;
struct tm tm;
bool time64;
+ int err_val;
+ bool check_tm;
} tests [] =
{
{"21:01:10 1999-1-31", "Universal", {10, 1, 21, 31, 0, 99, 0, 0, 0},
- false },
+ false , 0, true},
{"21:01:10 1999-1-31", "Universal", {10, 1, 21, 31, 0, 99, 0, 0, 0},
- false },
+ false , 0, true},
{" 21:01:10 1999-1-31", "Universal", {10, 1, 21, 31, 0, 99, 0, 0, 0},
- false },
+ false , 0, true},
{"21:01:10 1999-1-31 ", "Universal", {10, 1, 21, 31, 0, 99, 0, 0, 0},
- false },
+ false , 0, true},
{" 21:01:10 1999-1-31 ", "Universal", {10, 1, 21, 31, 0, 99, 0, 0, 0},
- false },
+ false , 0, true},
{"21:01:10 1999-2-28", "Universal", {10, 1, 21, 28, 1, 99, 0, 0, 0},
- false },
+ false , 0, true},
{"16:30:46 2000-2-29", "Universal", {46, 30,16, 29, 1, 100, 0, 0, 0},
- false },
- {"01-08-2000 05:06:07", "Europe/Berlin", {7, 6, 5, 1, 7, 100, 0, 0, 0},
- false },
+ false , 0, true},
+ {"01-08-2000 05:06:07", "Europe/Berlin", {7, 6, 5, 1, 7, 100, 0, 0, 0},
+ false , 0, true},
+ {"01-08-2000 a 05:06:07", "Europe/Berlin", {7, 6, 5, 1, 7, 100, 0, 0, 0},
+ false , 7, false},
+ {" 12 AM ", "Europe/Berlin", {7, 6, 5, 1, 7, 100, 0, 0, 0},
+ false , 0, false},
/* 64 bit time_t tests. */
{"21:01:10 2038-1-31", "Universal", {10, 1, 21, 31, 0, 138, 0, 0, 0},
- true },
+ true , 0, true},
{"22:01:10 2048-5-20", "Universal", {10, 1, 22, 20, 4, 148, 0, 0, 0},
- true },
+ true , 0, true},
{"01-08-2038 05:06:07", "Europe/Berlin", {7, 6, 5, 1, 7, 138, 0, 0, 0},
- true },
+ true , 0, true},
{"20-03-2050 21:30:08", "Europe/Berlin", {8, 30, 21, 20, 2, 150, 0, 0, 0},
- true }
+ true , 0, true}
};
static const char *
@@ -93,7 +99,8 @@ report_date_error (void)
static char *datemsk;
static const char datemskstr[] =
"%H:%M:%S %F\n"
- "%d-%m-%Y %T\n";
+ "%d-%m-%Y %T\n"
+ "%I %p\n";
static void
do_prepare (int argc, char **argv)
@@ -115,13 +122,23 @@ do_test (void)
setenv ("TZ", tests[i].tz, 1);
tm = getdate (tests[i].str);
- TEST_COMPARE (getdate_err, 0);
- if (getdate_err != 0)
+
+ /* Only check getdate_err when tm is NULL as getdate doesn't set
+ getdate_err on success. */
+ if (tm == NULL)
+ TEST_COMPARE (getdate_err, tests[i].err_val);
+ if (tests[i].err_val != 0) /* Expected failure */
+ {
+ TEST_COMPARE_BLOB (tm, 0, NULL, 0);
+ continue;
+ }
+
+ if (tm == NULL && getdate_err != tests[i].err_val)
{
support_record_failure ();
printf ("%s\n", report_date_error ());
}
- else
+ else if (tests[i].check_tm)
{
TEST_COMPARE (tests[i].tm.tm_mon, tm->tm_mon);
TEST_COMPARE (tests[i].tm.tm_year, tm->tm_year);
@@ -132,8 +149,9 @@ do_test (void)
}
struct tm tms;
- TEST_COMPARE (getdate_r (tests[i].str, &tms), 0);
- if (getdate_err == 0)
+ int retval = getdate_r (tests[i].str, &tms);
+ TEST_COMPARE (retval, tests[i].err_val);
+ if (retval == tests[i].err_val && tests[i].check_tm)
{
TEST_COMPARE (tests[i].tm.tm_mon, tms.tm_mon);
TEST_COMPARE (tests[i].tm.tm_year, tms.tm_year);
--
2.39.2
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] tst-getdate: Improve testcase flexibility and add test.
2023-06-09 15:59 [PATCH] tst-getdate: Improve testcase flexibility and add test Joe Simmons-Talbott
@ 2023-06-09 17:20 ` Arjun Shankar
2023-06-09 17:52 ` Joe Simmons-Talbott
0 siblings, 1 reply; 3+ messages in thread
From: Arjun Shankar @ 2023-06-09 17:20 UTC (permalink / raw)
To: Joe Simmons-Talbott; +Cc: libc-alpha
Hi Joe,
I have some comments -- but overall, this seems to improve coverage
and flexibility. Martin Coufal was interested in adding more getdate
tests and this change should make it easy to add some more odd cases.
> The getdate testcases all expect successful results. Add support for
> negative testcases and testcases where a full date and time are not
> supplied by skipping the tm checks in the test. Add a testcase that
> would catch a use-after-free that was recently found.
OK.
> diff --git a/time/tst-getdate.c b/time/tst-getdate.c
> index 4c9ed28d58..0036d313d7 100644
> --- a/time/tst-getdate.c
> +++ b/time/tst-getdate.c
> @@ -32,34 +32,40 @@ static const struct
> const char *tz;
> struct tm tm;
> bool time64;
> + int err_val;
> + bool check_tm;
OK.
err_val has the expected error value for each test case. This should
be 0 for all existing tests.
check_tm says if the test case should include checking values for tm.
This is because time-only inputs will return the current date, which
changes from run to run. This should be true for all existing tests.
> } tests [] =
> {
> {"21:01:10 1999-1-31", "Universal", {10, 1, 21, 31, 0, 99, 0, 0, 0},
> - false },
> + false , 0, true},
> {"21:01:10 1999-1-31", "Universal", {10, 1, 21, 31, 0, 99, 0, 0, 0},
> - false },
> + false , 0, true},
> {" 21:01:10 1999-1-31", "Universal", {10, 1, 21, 31, 0, 99, 0, 0, 0},
> - false },
> + false , 0, true},
> {"21:01:10 1999-1-31 ", "Universal", {10, 1, 21, 31, 0, 99, 0, 0, 0},
> - false },
> + false , 0, true},
> {" 21:01:10 1999-1-31 ", "Universal", {10, 1, 21, 31, 0, 99, 0, 0, 0},
> - false },
> + false , 0, true},
> {"21:01:10 1999-2-28", "Universal", {10, 1, 21, 28, 1, 99, 0, 0, 0},
> - false },
> + false , 0, true},
OK. Existing tests all get err_val = 0 and check_tm = true.
> {"16:30:46 2000-2-29", "Universal", {46, 30,16, 29, 1, 100, 0, 0, 0},
> - false },
> - {"01-08-2000 05:06:07", "Europe/Berlin", {7, 6, 5, 1, 7, 100, 0, 0, 0},
> - false },
> + false , 0, true},
> + {"01-08-2000 05:06:07", "Europe/Berlin", {7, 6, 5, 1, 7, 100, 0, 0, 0},
> + false , 0, true},
First test in this block is an old test. It gets a 0/true like others. OK.
Second test in this block looks like it got some new whitespace. Was
it intentional? I guess it's still OK because it touches upon
whitespace handling.
> + {"01-08-2000 a 05:06:07", "Europe/Berlin", {7, 6, 5, 1, 7, 100, 0, 0, 0},
> + false , 7, false},
> + {" 12 AM ", "Europe/Berlin", {7, 6, 5, 1, 7, 100, 0, 0, 0},
> + false , 0, false},
OK. Two new tests here:
1. has a spurious "a" in it and therefore should fail. Reason for
failure is that it doesn't match any of the date masks supplied.
2. has quite a bit of whitespace and is a time-only input. It should
succeed (err_val = 0), but we don't check the tm (check_tm = false)
because the date will be different at each execution.
>
> /* 64 bit time_t tests. */
> {"21:01:10 2038-1-31", "Universal", {10, 1, 21, 31, 0, 138, 0, 0, 0},
> - true },
> + true , 0, true},
> {"22:01:10 2048-5-20", "Universal", {10, 1, 22, 20, 4, 148, 0, 0, 0},
> - true },
> + true , 0, true},
> {"01-08-2038 05:06:07", "Europe/Berlin", {7, 6, 5, 1, 7, 138, 0, 0, 0},
> - true },
> + true , 0, true},
> {"20-03-2050 21:30:08", "Europe/Berlin", {8, 30, 21, 20, 2, 150, 0, 0, 0},
> - true }
> + true , 0, true}
> };
>
> static const char *
OK. All existing 64 bit time_t tests get 0/true in line with other
existing tests.
> @@ -93,7 +99,8 @@ report_date_error (void)
> static char *datemsk;
> static const char datemskstr[] =
> "%H:%M:%S %F\n"
> - "%d-%m-%Y %T\n";
> + "%d-%m-%Y %T\n"
> + "%I %p\n";
>
OK. New date mask added for the time-only test.
> static void
> do_prepare (int argc, char **argv)
> @@ -115,13 +122,23 @@ do_test (void)
> setenv ("TZ", tests[i].tz, 1);
>
> tm = getdate (tests[i].str);
> - TEST_COMPARE (getdate_err, 0);
> - if (getdate_err != 0)
> +
> + /* Only check getdate_err when tm is NULL as getdate doesn't set
> + getdate_err on success. */
> + if (tm == NULL)
> + TEST_COMPARE (getdate_err, tests[i].err_val);
OK. Just confirmed from the man-page:
"When successful, getdate() returns a pointer to a struct tm.
Otherwise, it returns NULL and sets the global variable getdate_err to
one of the error numbers shown below."
i.e. getdate_err isn't guaranteed to be reset to 0 upon success.
> + if (tests[i].err_val != 0) /* Expected failure */
> + {
> + TEST_COMPARE_BLOB (tm, 0, NULL, 0);
> + continue;
> + }
I think TEST_COMPARE_BLOB (x, 0, y, 0) is a nop.
> +
> + if (tm == NULL && getdate_err != tests[i].err_val)
> {
> support_record_failure ();
> printf ("%s\n", report_date_error ());
> }
Maybe this should go in along with the above check where we already do
TEST_COMPARE (getdate_err, tests[i].err_val)?
> - else
> + else if (tests[i].check_tm)
> {
> TEST_COMPARE (tests[i].tm.tm_mon, tm->tm_mon);
> TEST_COMPARE (tests[i].tm.tm_year, tm->tm_year);
OK. Now we conditionally check.
> @@ -132,8 +149,9 @@ do_test (void)
> }
>
> struct tm tms;
> - TEST_COMPARE (getdate_r (tests[i].str, &tms), 0);
> - if (getdate_err == 0)
> + int retval = getdate_r (tests[i].str, &tms);
> + TEST_COMPARE (retval, tests[i].err_val);
OK. Now we handle expected failures here.
> + if (retval == tests[i].err_val && tests[i].check_tm)
> {
> TEST_COMPARE (tests[i].tm.tm_mon, tms.tm_mon);
> TEST_COMPARE (tests[i].tm.tm_year, tms.tm_year);
OK. We conditionally check.
Thanks!
Arjun
--
Arjun Shankar
he/him/his
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] tst-getdate: Improve testcase flexibility and add test.
2023-06-09 17:20 ` Arjun Shankar
@ 2023-06-09 17:52 ` Joe Simmons-Talbott
0 siblings, 0 replies; 3+ messages in thread
From: Joe Simmons-Talbott @ 2023-06-09 17:52 UTC (permalink / raw)
To: Arjun Shankar; +Cc: libc-alpha
On Fri, Jun 09, 2023 at 07:20:38PM +0200, Arjun Shankar wrote:
> Hi Joe,
>
> I have some comments -- but overall, this seems to improve coverage
> and flexibility. Martin Coufal was interested in adding more getdate
> tests and this change should make it easy to add some more odd cases.
Hi Arjun,
Thanks for the review. I'll fix up the patch based on your comments and
post a v2 soon.
...
>
> > {"16:30:46 2000-2-29", "Universal", {46, 30,16, 29, 1, 100, 0, 0, 0},
> > - false },
> > - {"01-08-2000 05:06:07", "Europe/Berlin", {7, 6, 5, 1, 7, 100, 0, 0, 0},
> > - false },
> > + false , 0, true},
> > + {"01-08-2000 05:06:07", "Europe/Berlin", {7, 6, 5, 1, 7, 100, 0, 0, 0},
> > + false , 0, true},
>
> First test in this block is an old test. It gets a 0/true like others. OK.
>
> Second test in this block looks like it got some new whitespace. Was
> it intentional? I guess it's still OK because it touches upon
> whitespace handling.
That should have been a new test with more whitespace. Fixed in v2.
...
>
> > + if (tests[i].err_val != 0) /* Expected failure */
> > + {
> > + TEST_COMPARE_BLOB (tm, 0, NULL, 0);
> > + continue;
> > + }
>
> I think TEST_COMPARE_BLOB (x, 0, y, 0) is a nop.
Fixed in v2.
>
> > +
> > + if (tm == NULL && getdate_err != tests[i].err_val)
> > {
> > support_record_failure ();
> > printf ("%s\n", report_date_error ());
> > }
>
> Maybe this should go in along with the above check where we already do
> TEST_COMPARE (getdate_err, tests[i].err_val)?
Moved in v2.
Thanks,
Joe
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-06-09 17:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-09 15:59 [PATCH] tst-getdate: Improve testcase flexibility and add test Joe Simmons-Talbott
2023-06-09 17:20 ` Arjun Shankar
2023-06-09 17:52 ` Joe Simmons-Talbott
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).