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

Convert CLI interface tests to not use subprocess.run #970

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

jfrost-mo
Copy link
Member

@jfrost-mo jfrost-mo commented Dec 6, 2024

This no longer uses subprocess.run for (almost) all of the tests, which makes it faster and more accurately measured for test coverage.

For this the main branch is split into functions in the first commit, then the second commit converts the tests to not use subprocess, and adds a couple extras.

Contribution checklist

Aim to have all relevant checks ticked off before merging. See the developer's guide for more detail.

  • Documentation has been updated to reflect change.
  • New code has tests, and affected old tests have been updated.
  • All tests and CI checks pass.
  • Ensured the pull request title is descriptive.
  • Conda lock files have been updated if dependencies have changed.
  • Attributed any Generative AI, such as GitHub Copilot, used in this PR.
  • Marked the PR as ready to review.

@jfrost-mo jfrost-mo added the cleanup Non-functional improvement label Dec 6, 2024
@jfrost-mo jfrost-mo added this to the Quest 5 (December 2024) milestone Dec 6, 2024
@jfrost-mo jfrost-mo self-assigned this Dec 6, 2024
Copy link
Contributor

github-actions bot commented Dec 6, 2024

Coverage

@jfrost-mo jfrost-mo changed the title Improve CLI interface tests Convert CLI interface tests to not use subprocess.run Dec 6, 2024
@jfrost-mo
Copy link
Member Author

GitHub Copilot was used in this PR.

@jfrost-mo jfrost-mo requested a review from jwarner8 December 6, 2024 12:07
@jfrost-mo jfrost-mo marked this pull request as ready for review December 6, 2024 12:07
Base automatically changed from 581_nicer_exception_messages to main December 10, 2024 09:41
This is significantly faster and provides more accurate test coverage
than running the commands as subprocessses.
Copy link
Contributor

@jwarner8 jwarner8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just requesting few more comments/explanation to check understanding/make code more accessible.

src/CSET/__init__.py Show resolved Hide resolved
@jfrost-mo jfrost-mo merged commit faef359 into main Dec 12, 2024
8 checks passed
@jfrost-mo jfrost-mo deleted the improve_cli_tests branch December 12, 2024 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Non-functional improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants