Details
Diff Detail
- Lint
Lint Skipped - Unit
Unit Tests Skipped
In case you wondered - this cannot be done as a global fixture. Even session-scoped fixtures are executed after methods like setup_method(). That's very unfortunate, but that's the current implementation of pytest. Therefore we can't even use tmpdir, monkeypatch and similar fixtures (not to mention that they can be called only from function-scoped fixtures).
Still not thrilled that we have to do this but it certainly removes the need for some ugly test code that would be present with some other solutions. Looks good overall, just some questions.
testing/test_bodhi_comment_directive.py | ||
---|---|---|
16 ↗ | (On Diff #264) | why do the setup method this way instead of the builtin setup_method? |
testing/test_resultsdb_directive.py | ||
59 ↗ | (On Diff #264) | Why is does the config need to be reset for every run here when it wasn't needed before? |
testing/test_bodhi_comment_directive.py | ||
---|---|---|
16 ↗ | (On Diff #264) | The Config is modified here (Config.reporting_enabled = True etc), and there's no cleanup code. The following modules receive an altered object, which can cause some hard-to-debug side effects. Monkeypatch takes care of that and resets the object after these tests finish. |
testing/test_resultsdb_directive.py | ||
59 ↗ | (On Diff #264) | Ditto as above, to avoid side effects in following modules. Each module should clean after itself. Or we need to take other measures (e.g. have a global fixture that resets config._config between different modules). |
I still don't like the fact that we have to do this (fs access for unit tests, not the code here) but as I have no better alternative suggestions, I can't complain :)
testing/test_bodhi_comment_directive.py | ||
---|---|---|
16 ↗ | (On Diff #264) | After talking over IRC - the reason for using the explicit fixtures instead of the built-in xunit style setup_method is that those built-in mehtods can't use monkeypatch or tmpdir |