opengeospatial / ets-wmts10

WMTS Test suite
Other
1 stars 3 forks source link

Bug fixes, GetFeatureInfo tests, RESTful tests #78

Closed cmorriscmps closed 10 months ago

cmorriscmps commented 1 year ago

Contains bug fixes as well as options to execute tests for the GetFeatureInfo operation and the REST encoding.

Summary of Changes:

wmtsFunctions.xml

WMTS_ETS.xml

owsFunctions.xml

bootstrap.xml

dstenger commented 1 year ago

This pull request will be on hold until https://github.com/opengeospatial/teamengine/pull/593 is merged and installed on Beta.

cmorriscmps commented 1 year ago

The reference implementation @dstenger provided was failing due to an issue with style selection in the Server.KVP.GET.GetTile.Valid.Tile.Transparency test. I've fixed it and the RI now passes.

cmorriscmps commented 11 months ago

I really have no idea what the purpose of wmts-auto.xml was supposed to be. It appeared to be a copy of WMTS_ETS.xml, except wmts-auto.xml includes other files with xinclude. It doesn't really make sense to maintain two copies of the file. The config.xml file references the ctl directory, which loads all of the ctl files. They are all loaded twice if wmts-auto.xml is included.

dstenger commented 10 months ago

The wmts-auto.xml file is used by the CtlController which is executed by the already mentioned REST interface and command line interface. In addition to the includes, graphics content (java.awt) is removed.

I just checked the file and it seems that all changes are required by the REST interface.

So, I propose to restore the file to make this pull requests merge able.

However, I agree that the implementation should reduce duplication. So, we could create a new issue with the aim to improve/simplify/merge the two main CTL scripts.

dstenger commented 10 months ago

@cmorriscmps I will restore wmts-auto.xmlwhen I merge the pull requests to make it workable with the REST interface again.

Also, I am currently doing a functional review of the pull request and come back to you if I have any questions.

dstenger commented 10 months ago

I did a functional review of the pull request and these are my results.

Questions:

This should be adjusted:

Further comments:

dstenger commented 10 months ago

@cmorriscmps Thank you for the good pull request again. However, there are some things which need to be adjusted. Can you please check the list under This should be adjusted: in comment https://github.com/opengeospatial/ets-wmts10/pull/78#issuecomment-1894208642. Also, the options to enable GetFeatureInfo and REST tests shall be renamed to Include GetFeatureInfo Tests (optional) and Include RESTful Encoding Tests (optional) in the UI. If it is not possible for you to work on the changes, please contact me as I can support you here.

cmorriscmps commented 10 months ago

I'll take a closer look and respond by the end of the week.

cmorriscmps commented 10 months ago

To answer the question about the "Fail early on schema validation errors" option, If the option is selected then the root test will validate the capabilities document against the schema and fail without executing any subtests if validation fails. So yes, it will speed up test runs if validation fails.

I agree that the upload button for a capabilities document and that the wmts-auto.xml file should be restored. I can update the pull request with these changes.

I also agree that the ATS needs to be updated to reflect the current state of the ETS. However, it has been out of date long before this pull request was introduced, so I don't think that updating it falls under the scope of this pull request.

dstenger commented 10 months ago

Thank you for updating the pull request. Most problems are solved, now. There are some small tasks which are still pending:

@cmorriscmps Are you still working on updating the pull request? Can you please give me a short update if you need any assistance?

cmorriscmps commented 10 months ago

@dstenger I'm done with the pull request.

I've fixed the optional test labels.

The Upload capability has been restored. There is no Upload button on the initial form, but if you leave the Capabilities URL field blank, it will present another form asking for a file upload.

Feel free to update any other wording however you and/or Gobe think is appropriate. I'll also leave updating the ATS doc and the pom.xml up to you.

You can accept the pull request and make additional changes in the opengeospatial repository, or I've given you permission to edit the pull request if you would prefer to make changes there.

dstenger commented 10 months ago

Thank you for updating the pull request. I solved the last two open tasks:

dstenger commented 10 months ago

Further, I created a new issue for an update of the ATS: https://github.com/opengeospatial/ets-wmts10/issues/83

ghobona commented 9 months ago

This PR has now been installed on the Beta validator.

https://cite.ogc.org/te2/