Add request parameters #54
Conversation
This has been used by the Archiver Appliance implementation.
| expected = 'dummy-url?pv=dummy&from=2001-01-01T01:01:00Z&to=2010-02-03T04:05:00Z' | ||
| assert constructed == expected | ||
|
|
||
| with mock.patch('requests.get') as mock_get: |
There was a problem hiding this comment.
Interesting that you decided to mock and call get_values() rather than calling _construct_url().
There was a problem hiding this comment.
Sorry, I mentioned this bit in the pull request description rather than the commit message!
I was trying to test whether providing request_params results in the correct URL being sent. Testing _construct_url() doesn't really check for this, as it is not part of the public interface.
There was a problem hiding this comment.
I know it's a bit strange that I mocked out _parse_raw_data(), but that's because these tests are applied to an abstract class. I didn't want to make the pull request too big!
There was a problem hiding this comment.
Sorry, so you did.
I think testing the public API is probably better.
|
The only question now is why has tox broken? |
Pull Request Test Coverage Report for Build 386
💛 - Coveralls |
The Fetcher class now provides the ability to set additional parameters used during archiver retrieval. This has been used by the AaFetcher (Archiver Appliance) subclass, in particular.
The motivation for this was to enable the retiredPVTemplate parameter to be set for Archiver Appliance retrieval.
I have also modified the unit test that checks the url construction to use the public method 'get_values', as I thought it better to test the public interface. Please let me know your opinion on this.