ImperialCollegeLondon / safedata_validator

Python tools to validate and publish datasets using the safedata metadata format.
https://safedata-validator.readthedocs.io/
MIT License
2 stars 4 forks source link

Added mocked test of status codes #19

Closed jacobcook1995 closed 2 years ago

jacobcook1995 commented 2 years ago

Added a basic test to check that errors are correctly raised when certain status codes are returned. It's fairly bare bones at the moment, but I thought it was useful functionality to add as I might want to include more complex http status code handling in future, and the test can be extended to cover future cases. It didn't seem worth it to me to test taxonomy_esearch and taxonomy_efetch separately as they are very similar, so the test tests both of them at once.

jacobcook1995 commented 2 years ago

Made the change you suggested, + switched to using valid input so that if mocking somehow broke down the test wouldn't still pass. I'll leave the pull request open for David to look at when he gets back from holiday

davidorme commented 2 years ago

It seems a bit strange to me to be using unittest.mock here, rather than using the pytest-mock extension, which provides the mocker fixture for use in unit tests. Digging into it, it seems like pytest-mock is actually just exposing a fixture interface to the mock library, which became part of the standard library as unittest.mock, so they are effectively just different APIs to the exact same functionality.

It seems like the unittest approach works , and the decorator helps emphasize what is being mocked in a test, but using a single framework seems cleaner? @dalonsoa - thoughts?

dalonsoa commented 2 years ago

I don't have strong feelings either way, to be honest. I've used unittest in many projects without issues, and in others I've used pytest-mock also fine. I don't think it matters as long as you are consistent, using ONLY one or the other within a project.

davidorme commented 2 years ago

@jacobcook1995 Could we switch to using the pytest-mock, then.