-
Notifications
You must be signed in to change notification settings - Fork 11
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
improve test coverage #227
Conversation
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.
Partial review
Co-authored-by: Oliver Sanders <[email protected]>
tests/unit/test_rose_stem_units.py
Outdated
assert result == ('foo', '', '', '', 'foo') | ||
|
||
|
||
def test_process_multiple_auto_opts(monkeypatch, get_StemRunner): |
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.
Docstring?
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 just test the code as is, it took me a few minutes to work out what it was trying to do. Now I don't like what it's trying to do: It doesn't raise an error if there is a mistake in the Rose user/global config.
I'm inclined to set up a new issue for that, what do you think?
Improve test coverage Co-authored-by: Oliver Sanders <[email protected]>
Improve test coverage Co-authored-by: Oliver Sanders <[email protected]>
Partial Response to #157, and partial replacement of #196
The aim from my point of view is to separate easy small changes from the need to refactor subprocess tests more thoroughly to work in a manner similar to Cylc's integration tests.
My checks on coverage suggest that this brings coverage from just over 95% to just under 99%
Check List
CONTRIBUTING.md
and added my name as a Code Contributor.setup.cfg
(andconda-environment.yml
if present).