HorlogeSkynet / archey4

:computer: Maintained fork of the original Archey (Linux) system tool
https://git.io/archey4
GNU General Public License v3.0
295 stars 37 forks source link

Clean up tests #76

Closed ingrinder closed 4 years ago

ingrinder commented 4 years ago

As has been discussed previously, the Configuration singleton can cause some problems in our tests. Additionally, I've noticed that our tests aren't very... unit-test-y -- they're closer to integration tests than unit tests. Hopefully, these changes mean:

Description

Reason and / or context

In addition to the stuff described above, this is also a direct step to #57 where I have an implementation going that could be rebased onto this branch, which would help significantly with the number of broken tests it caused...

How has this been tested ?

There are some quite nice tests for the entry_mock helper method in its __init__.py. As for the decorator, I've yet to figure out exactly how to test that... Either way, all the other tests pass, of course. (The ones currently failing are ones I've added, none of the existing tests are broken).

Types of changes :

This is a bit hard to answer, these are really meta-changes being entirely about the tests.

Checklist :

ingrinder commented 4 years ago

Nice! I like that solution, it's very clean.

While having a look through I've noticed that our Configuration._config's sub-dictionaries are mutable when we get them with .get() or using dict() on the instance (ie with your new __iter__ method)...:

Mutating sub-dicts with `.get` ```python >>> class Gettable: ... def __init__(self): ... self._dictionary = {'subdict': {'somekey': 'somevalue'}} ... def get(self, key, default=None): ... return self._dictionary.get(key, default) ... >>> instance = Gettable() >>> instance._dictionary {'subdict': {'somekey': 'somevalue'}} >>> sub_dict_from_get = instance.get('subdict') >>> sub_dict_from_get {'somekey': 'somevalue'} >>> sub_dict_from_get['somekey'] = 'newvalue' >>> sub_dict_from_get {'somekey': 'newvalue'} >>> instance <__main__.Gettable object at 0x7ffa14a53dc0> >>> instance._dictionary {'subdict': {'somekey': 'newvalue'}} ```
Mutating sub-dicts with `__iter__` ```python >>> class Dictable: ... def __init__(self): ... self._dictionary = {'subdict': {'somekey': 'somevalue'}} ... def __iter__(self): ... return iter(self._dictionary.items()) ... >>> instance = Dictable() >>> instance._dictionary {'subdict': {'somekey': 'somevalue'}} >>> dict_from_iter = dict(instance) >>> dict_from_iter {'subdict': {'somekey': 'somevalue'}} >>> dict_from_iter['subdict']['somekey'] = 'newvalue' >>> dict_from_iter {'subdict': {'somekey': 'newvalue'}} >>> instance._dictionary {'subdict': {'somekey': 'newvalue'}} ```

Not ideal, since entries could unintentionally modify it, right? I might have a play about in a new branch changing our behaviour so that only Configuration itself can modify any configuration using its own internal methods.

Anyway, this PR looks good now, I'll go ahead and squash-rebase 👍

Edit: Oh, okay, GH only does squash + merge. That works too 😆