Refactor tests to use workflow generators#372
Conversation
28d82d8 to
77fd293
Compare
|
@gerrod3 Sorry I didn't get a chance to look deeply at this before going on PTO, I'll make sure to do so when I get back on Monday |
| WorkflowStep as WS, | ||
| repo_api, | ||
| pub_api, | ||
| distro_api, |
There was a problem hiding this comment.
edit: Hold off on making any changes yet, let's get #375 merged first and we can look at these changes later. We will need to put the compatibility release for pulpcore 3.13 out shortly and probably shouldn't wait on this PR.
going to stop my review here for now
These feel a bit over-shortened, and I think since most other plugins use
publication_api,distribution_apiand so forth, we should probably continue the pattern. An outside developer should be able to look at the code and understand what's going on without knowing the shorthands.
|
@dralley What do you think of doing something like what pulp_ansible does? https://github.com/pulp/pulp_ansible/blob/master/pulp_ansible/tests/functional/utils.py#L142-L158 |
|
+1. The migration plugin does something like that also. |
fea328a to
5fa8224
Compare
| def test_01_sync(self): | ||
| """Assert that syncing the repository triggers auto-publish and auto-distribution.""" | ||
| self.assertEqual(self.publications_api.list().count, 0) | ||
| self.assertEqual(self.pub_api.list().count, 0) |
There was a problem hiding this comment.
I do think we should stick to publications_api. Partly for consistency with other plugins, and partly because unnecessary shorthands make it harder for the uninitiated to understand what's going on. e.g. it would be great if external contributors could read our tests and understand / contribute to them easily.
| cls.content_api = content_api | ||
|
|
||
|
|
||
| class SyncHelpersMixin: |
There was a problem hiding this comment.
I would go with TestHelpersMixin since it has helpers for more than just sync
|
I like this approach a lot more than the DSL approach from previously, and it looks like it removes more code as well. Thanks! See comments, otherwise I think this looks fine. |
67b9639 to
74b6390
Compare
74b6390 to
0d1a437
Compare
2 participants
[noissue]