Add unit and functional tests of the SAL client #70
Conversation
TOFarmer
added
Priority: 3
TOFarmer
changed the title
| WHEN | ||
| A client performs a get operation | ||
| THEN | ||
| The client returns either a Branch or a DataObject |
There was a problem hiding this comment.
question: The actual test client only returns a Branch, and only a Branch is ever checked for. Is this specification overbroad, or is the test not fully implemented?
There was a problem hiding this comment.
Good point, I must have forgotten to add the parametrization for testing a DataObject. I've done so using a Scalar and I've also added the same type of parametrization for list and put, as these both really should apply to both the contents of branches and leaves, and there are minor differences in how the SALClient handles this.
| WHEN | ||
| The client performs a copy operation from a source to a target | ||
| THEN | ||
| A POST request |
There was a problem hiding this comment.
question: What does the POST request do?
There was a problem hiding this comment.
Whoops, I think this one had been left incomplete. I've updated it.
sal/tests/client/conftest.py
Outdated
| 'host': 'https://sal.testing/', | ||
| 'api': {'version': 2, | ||
| 'requires_auth': True, | ||
| 'resources': ['data', 'auth'], | ||
| 'classes': {'core': ['dictionary', | ||
| 'scalar', | ||
| 'string', | ||
| 'array']}}, | ||
| 'service': {'name': 'Simple Access Layer (SAL) Server', | ||
| 'version': '1.2.2'}, | ||
| 'request': {'url': 'https://sal.testing/'}} |
There was a problem hiding this comment.
suggestion: https://sal.testing is hard-coded twice in this fixture, would it be possible to use the host fixture instead?
sal/tests/client/unit/test_auth.py
Outdated
| cred_file = {'https://sal.testing': {'user': UN, | ||
| 'password': PW}} |
There was a problem hiding this comment.
suggestion: As in conftest.py, the host address is hard coded; the fixture could be used instead.
This situation occurs twice more in this file.
There was a problem hiding this comment.
Thanks, changed all three.
sal/tests/client/unit/test_host.py
Outdated
| The client raises a ConnectionError | ||
| """ | ||
|
|
||
| server_root_response.json.return_value['api']['version'] = 3 |
There was a problem hiding this comment.
suggeston: Instead of hard coding the future API version, would it be possible to get the current version and just add one?
There was a problem hiding this comment.
Good idea, I've implemented this.
Description of work
Add unit and functional tests of the SAL client using pytest.
All tests are documented using GIVEN-WHEN-THEN, and I would recommend this approach is used for all future tests.
All tests which are marked as expected fails have corresponding issues:
SALClient.copydoes not copy from source #68 (test_copy)Fixes
Fixes #65
To test
Reviewer checklist