opengeospatial / ets-ogcapi-processes10

Other
3 stars 3 forks source link

'null' or empty reason given for certain test failures #11

Closed pcdion closed 1 year ago

pcdion commented 2 years ago

for ogcapi-processes-1.0 test, Test Job Creation Op, Test Job Success return 'null' as reason for failure Test Job Creation Sync Raw Value One, and Test Job Creation Input Validation : Empty reason for failure

dstenger commented 2 years ago

Thank you for reporting. We will do further investigation. Is there a test service available we can use to reproduce the behavior?

jerstlouis commented 2 years ago

@dstenger not at the moment, sorry. This experimental version of our GNOSIS Map Server has not yet been integrated into our deployed software.

bpross-52n commented 2 years ago

This should be fixed in the current master branch.

dstenger commented 2 years ago

@jerstlouis Can you please check if this problem is still occurring when using the current status of master branch?

jerstlouis commented 1 year ago

@dstenger Just re-tested what I believe is the latest from master if my docker-fu was right (says Test version: 0.4-SNAPSHOT).

Something is still quite off, because now our implementation passes the Core conformance class, but with only 3 tests executed / passed, and 47 of them skipped, all showing "No details available".

The 3 tests that are executed / pass are:

It should not be possible to pass while skipping several of these tests that are required by Core.

We do have the following in the conformance resource:

The ETS did not attempt to request /processes. It seems like it did request /processes/echo but provided no feedback about it.

If necessary, we could try to set up an alternate GNOSIS Map Server URL with the EchoProcess and the experimental work trying to conform to Processes - Part 1.

dstenger commented 1 year ago

@jerstlouis Thank you for testing. We will discuss your results in the CITE team and come back to you.

gfenoy commented 1 year ago

We have found the following message node appearing in the testng-results.xml file:

<![CDATA[Could set up endpoint: http://macbook-pro-de-gerald-1/ogc-api//processes. Exception: Failed to load document from 'https://raw.githubusercontent.com/opengeospatial/ogcapi-processes/master/core/openapi/responses/LandingPage.yaml']]>

This LandingPage.yaml is referenced from this openapi.yaml but it is no more available (it was removed during the SWG meeting on 2023-01-23), but it is the file referenced from CommonFixture.java. This is the reason why the test suite does not run any tests depending on this OpenAPI file validation.

So, I did try to use the official ogcapi-processses-1.yaml. Unfortunately, it contains a small typo, if I understand correctly, which makes it not usable as-is. In consequence I have made the following copy here which solve the typo.

To be able to test your server, one solution is then to rely on this file. To do so, you have to modify the CommonFixture.java file locally with the following URI, then to rebuild the Docker image and run the tests:

specURI = new URI("https://raw.githubusercontent.com/GeoLabs/ogcapi-processes/proposal-dru-updates/openapi.yaml");

On our side it solved the issue and we get other tests running.

side-note: I also tried the following URI, but without success: https://raw.githubusercontent.com/opengeospatial/ogcapi-processes/master/openapi/ogcapi-processes.yaml

jerstlouis commented 1 year ago

@gfenoy Thanks for looking into this, and sorry for breaking this :)

Is this reference to the online resource at runtime? I would certainly not expect the test sutie to rely on online resources when running it (other than the implementation being tested of course).

Regarding the typo, the only difference between your "solved typo" is removing the |- and new line. I am not a YAML expert, and I am always confused by this particular syntax, but I believe this is proper YAML? YAMLLint thinks so as well.

Regarding the new openapi/ogcapi-processes.yaml, this file includes others with $ref, so perhaps this is the issue. There is also a bundled one, but is JSON, not sure if that is OK? However, I am not sure exactly what the test suite expects from this OpenAPI definition, there might meaningful differences.

In the meantime, we will give your work-around a try to make progress on our conformance testing.

gfenoy commented 1 year ago

Is this reference to the online resource at runtime? I would certainly not expect the test sutie to rely on online resources when running it (other than the implementation being tested of course).

Yes, the openapi.yaml file is downloaded at runtime.

Regarding the typo, the only difference between your "solved typo" is removing the |- and new line. I am not a YAML expert, and I am always confused by this particular syntax, but I believe this is proper YAML? YAMLLint thinks so as well.

When we tried using the official YAML file, we noticed that we got the same messages: Exception: Failed to load document referring to the file itself. In consequence, we tried using swaggerhub for loading the file and we got the information about the issue: "bad indentation of a mapping entry" on line 3. When we tried using - in place of the : in the title, it was reported as a valid file. Trying with the syntaxe shown in the file fixed, also solved the issue.

Regarding the new openapi/ogcapi-processes.yaml, this file includes others with $ref, so perhaps this is the issue. There is also a bundled one, but is JSON, not sure if that is OK? However, I am not sure exactly what the test suite expects from this OpenAPI definition, there might meaningful differences.

When looking back into this ogcapi-processes.yaml, we noticed the same issue with : used in title. Using this slightly modified file implies that we don't have the issue that the file cannot be loaded. Nevertheless, it implies also having a java.lang.NullPointerException in:

So we decided to move on with https://raw.githubusercontent.com/GeoLabs/ogcapi-processes/fix-failed-loading-openapi/openapi.yaml by now.

In the meantime, we will give your work-around a try to make progress on our conformance testing.

I hope you can confirm that the work-around works also on your side.

jerstlouis commented 1 year ago

@gfenoy Confirming that the work around does work in getting tests executed with our implementation, thanks again for helping us make progress.

Yes, the openapi.yaml file is downloaded at runtime.

This absolutely should not be the case. I would not expect the testing suite to rely on any external resources. It should be possible to run the test suite against a local Processes implementation completely offline (e.g., in a secure environment completely disconnected from the internet). Should a separate issue be filed for this?

When we tried using - in place of the : in the title, it was reported as a valid file.

Regarding the description |- at ogcapi-processes-1.yaml here on S/O I read that "there are NINE (or 63*, depending how you count) different ways to write multi-line string in YAML", and I agree that it is crazy (I should resume attempts to popularize ECON that does multiline in a simple way: simply close the double-quotes and reopen them on the next line, just like in C/C++/eC/etc.). So while I sympathize with the confusion, it still seems that it is valid syntax and all the tools confused by it are buggy:

Use >- or |- instead if you don't want a linebreak appended at the end.

Not sure if this is something that could easily be fixed in the test suite.

gfenoy commented 1 year ago

This absolutely should not be the case. I would not expect the testing suite to rely on any external resources. It should be possible to run the test suite against a local Processes implementation completely offline (e.g., in a secure environment completely disconnected from the internet). Should a separate issue be filed for this?

In my opinion it is a great idea to have a separate issue for this.

jerstlouis commented 1 year ago

Our latest Processes conformance test is now integrated and this failure can be tested directly from https://maps.gnosis.earth/ogcapi (Gerald's workaround is needed -- sorry for taking so long to provide this)

ghobona commented 1 year ago

@jerstlouis

We have updated the ETS and have set up a demo of OGC API - Processes using ZOO Project, with support from @gfenoy.

I am now testing GNOSIS, using the code in the master branch.

Please keep GNOSIS constant and unchanged as I review both the API and the ETS.

ghobona commented 1 year ago

Gerald's workaround of the ETS is out of date. Please use the ETS provided by the master branch of this repo.

jerstlouis commented 1 year ago

@ghobona After pulling the latest from master, Gerald's workaround is no longer necessary. This particular issue may be fixed, but still difficult to say due to remaining issues such as #12.

jerstlouis commented 1 year ago

Getting java.lang.AssertionError: null again after tweaks to process descriptions for tests Job Creation Op and Job Success.