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

conftest: add input arg for run_cli_command, fix typings #5175

Closed

Conversation

dev-zero
Copy link
Contributor

Additional arg "input" may be needed when testing interactive commands. For the typings: PEP484 mandates explicit Optional for optional args.

Additional arg "input" may be needed when testing interactive commands. For the typings: PEP484 mandates explicit Optional for optional args.
@dev-zero
Copy link
Contributor Author

To convert the unittests in my plugin to fixture-based (instead of the deprecated aiida test class) I copied the run_cli_command. Maybe the added input I needed is important to someone else who does testing of interactive cli.
What do you think about making those fixtures available as pytest plugins?

@codecov
Copy link

codecov bot commented Oct 12, 2021

Codecov Report

Merging #5175 (69c79e4) into develop (ddad09c) will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5175      +/-   ##
===========================================
+ Coverage    80.90%   80.92%   +0.03%     
===========================================
  Files          536      536              
  Lines        37054    37054              
===========================================
+ Hits         29975    29984       +9     
+ Misses        7079     7070       -9     
Flag Coverage Δ
django 75.78% <ø> (+0.03%) ⬆️
sqlalchemy 74.93% <ø> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aiida/orm/nodes/process/calculation/calcjob.py 78.00% <0.00%> (+0.96%) ⬆️
aiida/engine/processes/calcjobs/tasks.py 65.78% <0.00%> (+1.01%) ⬆️
aiida/engine/processes/calcjobs/manager.py 86.73% <0.00%> (+3.54%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ddad09c...69c79e4. Read the comment docs.

@chrisjsewell
Copy link
Member

Heya, FYI, I actually have done similar in #5172 (I missed this PR!):

def _run_cli_command(command: click.Command, options: list = None, raises: bool = False, **kwargs) -> Result:

It also handles catch_exceptions = False, which was present in the original AiidaTestCase

@sphuber
Copy link
Contributor

sphuber commented Oct 28, 2021

Thanks @dev-zero . As @chrisjsewell mentioned, I had already added this in #5111 which was merged today. I included your changes for the typing. So I am closing this PR.

@sphuber sphuber closed this Oct 28, 2021
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.

3 participants