Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: centralize FakeKojiSession definition #145

Open
ktdreyer opened this issue Apr 17, 2020 · 2 comments
Open

tests: centralize FakeKojiSession definition #145

ktdreyer opened this issue Apr 17, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@ktdreyer
Copy link
Owner

ktdreyer commented Apr 17, 2020

Every unit test has its own FakeKojiSession class, and its own "session" pytest fixture method. Each unit test file's FakeKojiSession class implements just enough of the Koji Hub API to pass the tests, but there is quite a bit of copy-and-paste here.

I think we've reached the point where we could centralize this into tests/conftest.py. We could define a single FakeKojiSession class and implement all the Koji Hub logic that we need into that class. A single "session" pytest fixture method could just return an instance of this class, and then all the unit test files can use this central fixture.

This would make our FakeKojiSession more complex, but we're at the point where there is considerable complexity already when we have to re-implement so much of the Hub's logic anyway. I think it would make it easier to get to full test coverage if we had a single large fixture. (I also wonder if we could extract this fixture into a completely standalone pytest plugin, for other Python projects that might want to fake calls to Koji).

@ktdreyer ktdreyer added the enhancement New feature or request label Apr 20, 2020
@ktdreyer ktdreyer changed the title tests: centralize FakeSession definition tests: centralize FakeKojiSession definition Jun 16, 2020
@ktdreyer
Copy link
Owner Author

I don't think I want to do this anymore. Instead I would probably focus on making the canonical Koji Hub code more reusable somehow.

@ktdreyer ktdreyer reopened this Nov 5, 2021
@ktdreyer
Copy link
Owner Author

ktdreyer commented Nov 5, 2021

After developing more koji-ansible features, which required writing even more improvements to FakeKojiSession (eg #249), I'm reconsidering this.

As we add more advanced features, the pytest suite also become more advanced. Here are my recent thoughts:

The simplest way to unit test new code is to pass in simple Mock objects as a session that accept any call parameters. Unfortunately this almost never catches Koji API parameter issues (eg. #250). For that, we must rely on the integration tests with live hubs. Those are harder for newcomers, much slower than unit testing, and it's difficult to exercise all corner cases when we have no code coverage reporting for the integration tests.

When FakeKojiSession is sufficiently advanced, it's really handy to scaffold complex tag and package configurations inside unit tests. For example, in the "block a package" unit tests, it's really nice to setup the environment like you would on a "live" hub, with session.packageListAdd('ceph-5.0-rhel-8', 'ceph', 'kdreyer'), and then unit-test blocking that session's "ceph" package.

Problems I've seen with FakeKojiSession:

  1. It's more complex for newcomers to write entirely new tests, because they need to implement some of Koji's server-side logic as well. It's much easier for a typical Python developer to deal with a Mock object and a static return_value.
  2. I have to keep the API parameters in sync "by hand" with hub/kojihub.py. I wonder if I could write some tool to analyze the koji hub API and keep the fake's API in sync?
  3. Each unit test module has its own implementation, depending on the features it needs. This sprawl and duplication grew organically and keeps each test simpler to read, but it makes it harder to share complex features across tests now. For example koji_tag, koji_tag_inheritance, and koji_tag_packages end up with FakeKojiSession implementations that are poor copy-and-pastes, and it slows down feature development when I have to implement the same logic in all three fakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant