| Message ID | 20230420160837.1083228-3-tytso@mit.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Work around various report.xml compatibility issues | expand |
On Thu, Apr 20, 2023 at 12:08:37PM -0400, Theodore Ts'o wrote: > Commit 545315976e72 ("report: capture the time zone in the test report > timestamp") in the fstests upstream adds a timezone to the timestamp. > This is useful, but it breaks the strptime parsing of the timestamp. > It's not entirely clear that adding a timezone is legal. According to > this schema[1] that it is a ISO 8061 date/time stamp where the > "Timezone may not be specified". However in this schema[2], the > timestamp is just an optional string, and it says nothing about the > format of the timestamp. > > [1] https://gist.github.com/jclosure/45d7005d120d90ba6430130356e4cd61#file-xunit-xsd-L140 > [2] https://github.com/junit-team/junit5/blob/main/platform-tests/src/test/resources/jenkins-junit.xsd#L96 > > It's not entirely clear which schema is "official", but in the spirit > of the second part of Jon Postel's Robustness principal --- "be > liberal in what you accept" --- fix gen_results_summary.py to accept > an ISO 8061 timestamp with or without the timestamp. FWIW I added the explicit UTC offset because without it, we have to assume local time. That works poorly when your fstests cloud is scattered around the world, because then the result timestamps go all over the place. > It might be that in the spirit of "be conservative in what you send", > fstests upstream should use the moral equivalent of 'date -u +"%F %T" | > sed -e "s/ /T/"' instead of 'date -Iseconds', but this change will > work either way, as well as with both the older and newer versions of > fstests. > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> Probably the only sane way to handle this, so... Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > .../usr/lib/python3/dist-packages/gen_results_summary.py | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/test-appliance/files/usr/lib/python3/dist-packages/gen_results_summary.py b/test-appliance/files/usr/lib/python3/dist-packages/gen_results_summary.py > index ecf5dc1a..44fb07d2 100644 > --- a/test-appliance/files/usr/lib/python3/dist-packages/gen_results_summary.py > +++ b/test-appliance/files/usr/lib/python3/dist-packages/gen_results_summary.py > @@ -61,8 +61,12 @@ def parse_timestamp(timestamp): > """Parse an ISO-8601-like timestamp as found in an xUnit file.""" > if timestamp == "": > return 0 > - return time.mktime(datetime.strptime(timestamp, > - '%Y-%m-%dT%H:%M:%S').timetuple()) > + for fmt in ('%Y-%m-%dT%H:%M:%S%z', '%Y-%m-%dT%H:%M:%S'): > + try: > + return time.mktime(datetime.strptime(timestamp, fmt).timetuple()) > + except ValueError: > + pass > + raise ValueError('no valid timestamp format found') > > def failed_tests(testsuite): > """This iterator the failed tests from the testsuite.""" > -- > 2.31.0 >
diff --git a/test-appliance/files/usr/lib/python3/dist-packages/gen_results_summary.py b/test-appliance/files/usr/lib/python3/dist-packages/gen_results_summary.py index ecf5dc1a..44fb07d2 100644 --- a/test-appliance/files/usr/lib/python3/dist-packages/gen_results_summary.py +++ b/test-appliance/files/usr/lib/python3/dist-packages/gen_results_summary.py @@ -61,8 +61,12 @@ def parse_timestamp(timestamp): """Parse an ISO-8601-like timestamp as found in an xUnit file.""" if timestamp == "": return 0 - return time.mktime(datetime.strptime(timestamp, - '%Y-%m-%dT%H:%M:%S').timetuple()) + for fmt in ('%Y-%m-%dT%H:%M:%S%z', '%Y-%m-%dT%H:%M:%S'): + try: + return time.mktime(datetime.strptime(timestamp, fmt).timetuple()) + except ValueError: + pass + raise ValueError('no valid timestamp format found') def failed_tests(testsuite): """This iterator the failed tests from the testsuite."""
Commit 545315976e72 ("report: capture the time zone in the test report timestamp") in the fstests upstream adds a timezone to the timestamp. This is useful, but it breaks the strptime parsing of the timestamp. It's not entirely clear that adding a timezone is legal. According to this schema[1] that it is a ISO 8061 date/time stamp where the "Timezone may not be specified". However in this schema[2], the timestamp is just an optional string, and it says nothing about the format of the timestamp. [1] https://gist.github.com/jclosure/45d7005d120d90ba6430130356e4cd61#file-xunit-xsd-L140 [2] https://github.com/junit-team/junit5/blob/main/platform-tests/src/test/resources/jenkins-junit.xsd#L96 It's not entirely clear which schema is "official", but in the spirit of the second part of Jon Postel's Robustness principal --- "be liberal in what you accept" --- fix gen_results_summary.py to accept an ISO 8061 timestamp with or without the timestamp. It might be that in the spirit of "be conservative in what you send", fstests upstream should use the moral equivalent of 'date -u +"%F %T" | sed -e "s/ /T/"' instead of 'date -Iseconds', but this change will work either way, as well as with both the older and newer versions of fstests. Signed-off-by: Theodore Ts'o <tytso@mit.edu> --- .../usr/lib/python3/dist-packages/gen_results_summary.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)