opengeospatial / ets-sta10

Repository for the Executable Test Suite for OGC Sensor Things API
Other
6 stars 8 forks source link

Should locations be compared as Strings? #35

Open albertopq opened 7 years ago

albertopq commented 7 years ago

I'm running into an issue when ordering by Locations.location. I think that should compare results by coordinates, but the test is comparing as a String, according to:

https://github.com/opengeospatial/ets-sta10/blob/master/src/main/java/org/opengis/cite/sta10/filteringExtension/Capability3Tests.java#L537

We are delegating that orderby to SQL (postgres) directly and it seems that the result is different:

coordinates: [-117.05, 51.05] < coordinates: [-100.05,50.05]

when the test is expecting the opposite.

Could somebody please elaborate on the idea behind this test? Thanks!

hylkevds commented 7 years ago

It should probably exclude Locations.location from the orderby tests, since ordering by location does not make sense. Especially when considering polygon locations. Currently there is only an exclusion of unitOfMeasurement. Thing.properties, Observation.parameters and FeatureOfInterest.feature should probably also be excluded from the sorting tests.

taniakhalafbeigi commented 7 years ago

Thanks for pointing that out. And yes, I agree. We'd better exclude all Thing.properties, Observation.parameters, FeatureOfInterest.feature, and Location.location from $orderby test.

hylkevds commented 7 years ago

I've fixed this in the last commit on the pull request https://github.com/opengeospatial/ets-sta10/pull/32 since it nicely fits in that set. Instead of just using a list of string, it introduces a class EntityProperty that has several fields that are useful for the checks. The commit also fixed several other places where properties where not tested quite right.

If you feel adventurous you could test our fork of the test suit: https://github.com/FraunhoferIOSB/ets-sta10

albertopq commented 7 years ago

That sounds good. What are the next steps then? When is the PR expected to land? Should I submit a smaller PR that only fixes that?

Thanks!

mariakrommyda commented 5 years ago

Hello everyone,

According to the discussion above the unitOfMeasurement, Thing.properties, Observation.parameters, FeatureOfInterest.feature, and Location.location should be excluded from the order by test. In line 320-322 though of the relevant file, ets-sta10/src/main/java/org/opengis/cite/sta10/filteringExtension/Capability3Tests.java, only the unitOfMeasurement is excluded. Am I missing something or this is a still pending correction?

Best regards, Maria.

dstenger commented 5 years ago

@taniakhalafbeigi Can you please take a look at this question? What is the current status of the test suite here?

taniakhalafbeigi commented 5 years ago

Currently there is no PR specific to this issue and this issue hasn't been addressed yet.