-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat!(test): add a pytest plugin #653
Conversation
This should make testing the Application more straightforward, as we're always in debug mode when we run tests (so `run()` raises exceptions that would otherwise become InternalErrors). Already caught a test issue :-)
This adds a pytest plugin for craft-application, with two fixtures: 1. debug_mode: This fixture puts the app into debug mode by default. 2. production_mode: Overrides debug_mode for a function. This is a breaking change because debug_mode is an auto-used fixture, meaning any tests that rely on production mode will break.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one, thanks!
from pyfakefs.fake_filesystem import FakeFilesystem | ||
|
||
|
||
@pytest.fixture(autouse=True, scope="session") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you scope to to session, wouldn't any use of production_mode
override it as it will be run only once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
production_mode
uses monkeypatch.setenv
, which cleans up after itself during teardown. Since production_mode
is function-scoped, it returns the CRAFT_DEBUG
environment variable to its initial state on completion of the test function.
If we imagine the fixtures as context managers, pytest is doing roughly the following:
with debug_mode():
test_that_doesnt_use_production_mode()
with production_mode():
production_mode_test()
another_debug_mode_test()
This is also why we're setting os.environ
directly here, as monkeypatch
is function-scoped.
If someone modified os.environ["CRAFT_DEBUG"]
directly in one of their own tests or fixtures, it would break this behaviour, but it would also break plenty of other assumptions pytest makes.
util.get_host_architecture.cache_clear() | ||
yield arch | ||
util.get_host_architecture.cache_clear() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about clearing it in the teardown part. Is it required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imagine these two tests:
def test_with_fake_arch(fake_host_architecture):
assert util.get_host_architecture() == fake_host_architecture
def test_with_real_arch():
assert util.get_host_architecture() == subprocess.run(
["dpkg", "--print-architecture"], text=True, capture_output=True
).stdout.strip()
Running both tests in definition order will cause test_with_real_arch
to fail in most cases (it will succeed if the last run of test_with_fake_arch
happened to be the real architecture), but running just test_with_real_arch
will always succeed.
Don't you love side-effects? 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General idea and implementation seems sound, but I haven't tested that separately. I'll trust the bundled tests.
make lint && make test
?docs/reference/changelog.rst
)?This adds a pytest plugin to craft-application with some basic fixtures that are useful both for craft-application itself and for applications that use it.
CRAFT-4158