Reorganize tests

[aurelg]

Problem 1

In client_test.py, the tests override the default __init__ function of each client with new_init. Among other things, this new_init overrides the real provider with a fake one which just echoes what it is being sent.

The return value of the post function of clients is consequently ambiguous:

• when called from the production code, it is tested against a boolean in feedspora_runner:197 (suggesting that it should be a boolean as well)
• when called from client_test.py, it is tested against a dict

This make the implementation of the post function unnecessarily complicated and fragile.

Problem 2

On top of this convoluted testing strategy, the post_test tests also define their own way to test the clients, by passing a testing parameter to their __init__. Consequently, some testing-related code has to be implemented in the code being tested, i.e. in the __init__ and post function of each client. This is also bad practice, unnecessarily complicated and fragile.

Solution

The usual solution to problem 2 is to monkeypatch the objects being tested. However, monkeypatching may cause ambiguous return types like problem 1 unless functions are specifically implemented to be easy and unambiguous to test. Since that's not the case here, this should be fixed.

[wilddeej]

Another aspect I recently noticed is that the check_feed helper function can only be called one time within a given test function. Any more than that and there are errors thrown regarding invalid JSON document reading. This makes it impossible to group multiple tests within a loop (like, a basic test group for all clients, as an example).

[aurelg]

check_feed fixed w/ b5b870b

[wilddeej]

Some additional random thoughts on this subject:

1. In general, the more "realistic" the testing is (as in, the more accurately the actual client-level operations are tested), the more useful those tests will be
2. Ideally, the test mechanisms used will not require - or at least should greatly reduce the requirement of - "double development"; code changes that require duplicating functionality in the test code are both tedious and error-prone. As an example, the TweepyClient tests require the same complicated formatting to be applied as the client does, making it that much more difficult to implement any formatting changes to this client.
3. The way it is implemented now, there is both fundamental client code testing, and testing of the various options as defined in the configuration files. I think this is actually pretty useful and a good strategy, although it could probably be implemented in such a way that two different mechanisms are not needed.
4. One thing that is not tested (but should be) is how/when data gets entered into the "published" DB, and neither class of testing that exists now is set up to provide that level.
5. Related to the above, all posting attempts during testing technically return a "failed" result from the client posting function. Not only does this prevent the entry from getting marked as "published", it also disables the code related to the max_posts option (both making this aspect untestable and necessarily making tests take longer than they should, since every feed item gets test-posted as a result). This also violates the first point above, as this is "less realistic" as compared to how a real run would perform/operate.
6. Adding an additional level of abstraction might be worth considering. Right now, the client post functionality takes care of all the necessary data formatting, then directly calls the relevant client posting function. If there were another level in between that had the same parameter list for all clients (perhaps kwargs for title, content, link, tags, and media at a minimum), and then THAT function took care of the lower-level posting duties, that mid-level function could be overridden for testing purposes. Just an idea...