-
-
Notifications
You must be signed in to change notification settings - Fork 241
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
Add basic cli tests for date/time options and fix build warnings #316
Conversation
669a509
to
582ffbd
Compare
@jmaupetit Thanks to you merging #315 I've been able to remove the unneeded commit in this PR, and it should be ready to be code-reviewed / merged. |
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.
Thanks for taking the time to do that!
The way you structured the tests is a bit unusual, at least to my eyes. Are you following a known pattern, or is it just something you came up with? :)
No judgment involved, I would be happy to refactor it in what I think would be a more conventional way if you'd like.
No problem!
I'm not sure I understand what you mean 😅 Is the hierarchy what looks unusual to you? I remember I programmed this trying to refactor code after adding each test in order to avoid duplicate code. In any case, there's no pattern here, just what I came up with
Please, go ahead! I'm sure the tests will be more consistent and thorough if you could give me a hand here 👍 |
@k4nar If it's of any help, I wouldn't mind refactoring the tests into a more usual structure if you pointed me to some example/reference code. Thank you for your time and excuse my insistence! |
582ffbd
to
448561f
Compare
Pytest does not recognize the 'datafiles' mark provided by the corresponding plugin. The solution is to register this mark as a custom one in pytest.ini, as suggested by the warning message itself.
448561f
to
851abe2
Compare
@k4nar: I've removed inheritance to make the tests more "pytesty" 🤞 |
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.
Great work thanks! Sorry for never taking the time to go back to you :( .
Inspired by tests in #282 (which is sadly stalled), here are some basic tests for testing date/time correctness for all Watson commands which do have time-related options.
Besides that, a mock deprecation warning has been fixed by using mocker. It was a trivial replace, that's why I've decided to add it to this PR, although in a separate commit.
The message in the builds was as follows:
A pytest warning related to the datafiles plugin has also been fixed. The message was as follows:
This PR also includes changes from PR #315 as they are necessary to run green. When that PR is merged, I will rebase and leave only the other commits, if that's OK for you☺️ Update 2019-09-30: This PR is now ready to be code-reviewed / merged.