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

Limit dependency on script runner fixture #85

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

diazona
Copy link
Owner

@diazona diazona commented Oct 3, 2023

This PR alters the Project class so we can create it without a ScriptRunner; now we pass the ScriptRunner to the run() or run_cli() method instead. This lets us avoid using the script_runner fixture in some places where it wasn't really necessary, and was only needed to have a parameter to pass to the Project constructor.

The Project class needs something to run scripts and programs, and up to
this point that has been the ScriptRunner class from
pytest-console-scripts. But that makes every single use of Project
dependent on the a script_runner fixture - unless we decide to start
constructing custom ScriptRunner instances, but that would be a little
awkward since that class expects to be given things like option values
for the console-scripts plugin. It would be much nicer if we had
the option to swap out the runner and potentially use something simpler.

This commit makes that change by defining a new protocol type which
describes exactly the behavior we need for a runner. That behavior is
provided by ScriptRunner.run() (or a simple wrapper around it, if using
an old version of pytest-console-scripts where the signature is
different), but now we can use other implementations as well.

In addition, this commit also defines a new protocol for the return type
of a runner. As with the runner protocol, the return type protocol is
compatible with the version from pytest-console-scripts but can also be
implemented with something custom.
This commit encapsulates the determination of which version of
ScriptRunner.run() to use in a new fixture.
This commit changes the Project class so that we provide a runner to
the run() or run_cli() method, rather than to the constructor. This
means we can now create a Project without a runner, and only provide one
as needed in tests which actually use that functionality. In particular,
the live distribution package tests have no more need to accept a runner
at all.
@diazona diazona added this to the Initial release milestone Oct 3, 2023
@diazona diazona requested a review from sjlongland October 3, 2023 05:49
@sjlongland sjlongland merged commit 7ce2a55 into main Oct 3, 2023
@sjlongland sjlongland deleted the break-dependency-on-script-runner-fixture/1/dev branch October 3, 2023 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants