Skip to content

add a null check before using attrib #978

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

pvgenuchten
Copy link
Contributor

resolves issue #973

@geographika
Copy link
Contributor

@pvgenuchten - I've added a test for this update based on the XML responses from the live server. Tests fail without your update, and pass with it. The PR was previously failing due to an unrelated reason (fixed in #982).

@tomkralidis - as discussed at #971 (comment) the test uses https://pytest-httpserver.readthedocs.io/ which allows request to be made by OWSLib, but using a mock server that can be configured to return XML from a saved file. It should be possible to run these in offline mode too (allowing only access to localhost) with a wrapper function in a conftest.py file - this can be addressed in a future PR.

There is already a similar approach in OWSLib of loading XML from a file, for example:

belgian_sample = str(Path(__file__).parent.parent / "tests" / "resources" / "iso3_examples" / "metawal.wallonie.be-catchments.xml")

The advantage of https://pytest-httpserver.readthedocs.io/ is that it allows for testing of classes that don't allow XML to be passed in directly, and allows for end-to-end testing.

Let me know your thoughts.

@pvgenuchten
Copy link
Contributor Author

Thanx for adding a test Seth

@tomkralidis tomkralidis merged commit ea64e17 into geopython:master Mar 8, 2025
3 checks passed
@tomkralidis
Copy link
Member

@pvgenuchten - I've added a test for this update based on the XML responses from the live server. Tests fail without your update, and pass with it. The PR was previously failing due to an unrelated reason (fixed in #982).

@tomkralidis - as discussed at #971 (comment) the test uses https://pytest-httpserver.readthedocs.io/ which allows request to be made by OWSLib, but using a mock server that can be configured to return XML from a saved file. It should be possible to run these in offline mode too (allowing only access to localhost) with a wrapper function in a conftest.py file - this can be addressed in a future PR.

There is already a similar approach in OWSLib of loading XML from a file, for example:

https://github.com/geopython/OWS.Lib/blob/39ad5a90c68925854424412a413b03728d38fcfa/tests/test_iso3_parsing.py#L302

The advantage of https://pytest-httpserver.readthedocs.io/ is that it allows for testing of classes that don't allow XML to be passed in directly, and allows for end-to-end testing.

Let me know your thoughts.

Thanks for the info @geographika. I think this would be a valuable update to our testing. Feel free to open a dedicated issue so we can plan (and maybe split up the work if needed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants