timeStamp failures on items responses

[tomkralidis]

Testing http://cite.opengeospatial.org/te2 (username tomkralidis, session s0078) against http://147.102.109.27:5001 yields 3 failures of type:

• java.lang.AssertionError: timeStamp in response must not be after the request was send (2020-01-02T16:11:52Z) but was '2020-01-02T16:11:52.430407Z'

[aaime]

I'm seeing the same running tests against GeoServer. Tracked down the test to this bit:

https://github.com/opengeospatial/ets-ogcapi-features10/blob/a7154544128b4ca219c46dc9e73557256a87f09b/src/main/java/org/opengis/cite/ogcapifeatures10/collections/FeaturesAssertions.java#L49

It seems it's checking local times against the remote times reported in the timestamp property of the response... if the responses are quick, even a little skew in clocks would make the checks fail, believe this test should not be run or at least have a significant tolerance.

Interestingly enough, I see it fail anyways while running the tests locally, and get an error message like:

timeStamp in response must not be after the request was send (2020-01-16T15:42:33Z) but was '2020-01-16T15:42:33.455Z'

The test is checking for a strict "after" using a precision of seconds, but if the server and network are fast enough, the response can come back within the same second it started...

[dstenger]

@tomkralidis Can you provide a public link to a service which can be used to reproduce the bug? The linked service in issue description does not work anymore.

[tomkralidis]

@dstenger you can now test against https://demo.pygeoapi.io/cite

[keshavnangare]

@dstenger

I went through the test implementation and able to find the root cause.

https://github.com/opengeospatial/ets-ogcapi-features10/blob/a7154544128b4ca219c46dc9e73557256a87f09b/src/main/java/org/opengis/cite/ogcapifeatures10/collections/FeaturesAssertions.java#L58-L64

We can see the variable timeStampAfterResponse at line no. 59, the seconds are getting truncated and validated with the assertion which causes this issue. It's not clear to me why seconds are truncated here.

timeStamp in response must not be after the request was send (2020-02-13T10:25:44Z) but was '2020-02-13T10:25:44.403847Z'

We can see the above error message i.e seconds are not present in the first DateTime object and present in the second object, this is the reason assertion get false value and it failed with the above error.

I have updated the test here and removed the truncation of secondes then test ran successfully, there is no error message regarding timestamp.

@dstenger @lgoltz

Can you please confirm if we need this truncate seconds logic for a specific reason? else will remove it and create PR for this change.

[lgoltz]

@keshav-nangare See https://github.com/opengeospatial/ets-ogcapi-features10/issues/80#issuecomment-563223232 for more information why the seconds are truncated. I think the comparison should include 'equal'.

[keshavnangare]

Thank you! @lgoltz

I have tried a comparison with equals and tested the changes with service, the timestamp issue is not occurring any more.

[dstenger]

As far as I see, the truncation of seconds is wrong and should be removed. Truncation of seconds should just be applied when "not before" is tested In both cases, the test is made more flexible/lax when removing truncation (not after) and using truncation (not before).

Using 'equal' is in my opinion not correct as 'equal' might be a valid result (very, very fast servers).

[aaime]

@dstenger the test is IMHO inherently brittle, it works under the assumption that the system under test is running on a machine that is perfectly synchronized with the one running the test itself (e.g., the official CITE tests server), a time shift of a few seconds between the two might make the test systematically fail.

[dstenger]

I agree. However, then we must define how lax the test should be: Minutes, hours, days etc.

[dstenger]

We propose following solution: The check comparing date-time of client (TEAM Engine) and server (instance under test) is removed as the precondition of exactly synchronized times is too strict. Instead, it is just checked if the value is a valid (parsable) date-time value.

[tomkralidis]

+1

[dstenger]

@keshav-nangare Can you please remove the two assertions as proposed in my previous comment? The only assertion remaining: parseAsDate( timeStamp );

[keshavnangare]

@dstenger I have removed the date assertion. Can you please verify the fix?