-
Notifications
You must be signed in to change notification settings - Fork 24
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 tests suite #120
Add tests suite #120
Conversation
Great PR! Gonna be really awesome to have a testing suite, and this one looks really comprehensive :)
I'm going to read up on tox, seeing as I don't currently know what it is. I'm assuming it helps catch errors that may crop up in the process of packaging and importing? |
using autouse fixture, instead of assertion.
after Ozone object instantiation
now all tests have their necessary VCRs
Some caveats about the tests and cassettes:
My suggestion to go about this:
@Milind220 Given we're both new to tox, I think we should focus on the test suite as it is and think about it in another time. Sorry for raising it earlier, it felt like I've bothered you and overcomplicated the PR unnecessarily. 🙏🏼 |
Oh yikes! Great that we have tests now though :)
I absolutely agree, tests will ensure that Ozone's code is up to a decent standard.
Let's leave the cassettes as they are, and fix the bugs asap for sure.
No worries, we can always add it in another time. Awesome work! I went through the tests and they look really solid and well formulated. Can't wait to see Ozone'd code quality improve with the tests checking :) |
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.
Tests look really great - I think we'll find a lot more wrong with Ozone - which is a good thing!
@pytest.mark.vcr | ||
def test_bad_coordinates(): | ||
with pytest.raises(Exception): | ||
api.get_range_coordinates_air(lower_bound=("lol", "bruh"), upper_bound=(0, 0)) |
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.
hahahaha 'lol' and 'bruh'
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.
- Short enough
- Not interpretable as a valid coordinate value
So ... hehe.
@lahdjirayhan Just to confirm, this PR is complete right? I do have a couple questions.
|
Yes. Feel free to merge if you think it's enough.
Yes. Each developer can verify and test their work locally and automatically (not manually test each cases one by one).
Yep. It's not included in this PR. |
This PR is currently working in progress, to fix #8.
List of public methods to write tests of:
get_city_air
get_city_forecast
get_city_station_options
get_coordinate_air
get_historical_data
get_multiple_city_air
get_multiple_coordinate_air
get_range_coordinates_air
get_specific_parameter
reset_token
?In summary:
pytest
as the test frameworkpytest-recording
plugin to utilizevcr.py
for mocking API response dataThis PR should also:
Potential discussion points:
tox
for doing the tests? Especially in light of error discovered in Ozone doesn't import, 'ModuleNotFoundError: No module named 'ozone.historical'' #119