-
Notifications
You must be signed in to change notification settings - Fork 3
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
Fix resource calls #16
Fix resource calls #16
Conversation
6a9d929
to
099f19d
Compare
Thanks for your contribution! |
099f19d
to
d82b4c8
Compare
Done, it's ok? |
I added some reviews in your tests, otherwise everything looks good ! |
d82b4c8
to
d4f8358
Compare
There are two errors: one due to I agree that for now |
tests/test_geoserver.py
Outdated
assert isinstance(data, bool) | ||
assert data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think something like the below is easier to read:
assert data is True
WDYT?
tests/test_geoserver.py
Outdated
def test_delete_resource(test_geoserver: GeoServer, test_workspace: str, test_coverage: str) -> None: | ||
path = f"data/{test_workspace}/{test_coverage}" | ||
msg = test_geoserver.delete_resource(path) | ||
assert isinstance(msg, str) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is problematic because it will delete the fixture test_coverage
, and the fixture teardown will break (because it will try to remove an already deleted coverage).
Instead I would suggest to upload a tmp coverage, and then delete it within the test.
def test_delete_resource(test_geoserver: GeoServer, test_workspace: str) -> None:
coveragestore = "tmp-coveragestore"
file_path = Path(TEST_DATA_DIR, "rasters", "raster.tif").resolve()
test_geoserver.upload_coverage_store(
file=file_path,
name=coveragestore,
workspace=test_workspace,
format="geotiff",
)
path = f"data/{test_workspace}/{coveragestore}"
msg = test_geoserver.delete_resource(path)
# ...
d4f8358
to
305b691
Compare
* test: add resource calls tests
3684d19
to
65fba01
Compare
Thanks @mcebria-hg for this PR and your effort & time! |
The calls to resources don't work, I have adapted them according to geoserver documentation and performed the call tests.