-
Notifications
You must be signed in to change notification settings - Fork 96
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
Refactor test_area
and move boundary related tests to test_area_boundary
#564
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #564 +/- ##
==========================================
- Coverage 94.09% 93.88% -0.21%
==========================================
Files 85 88 +3
Lines 13235 13342 +107
==========================================
+ Hits 12453 12526 +73
- Misses 782 816 +34
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Looks good to me!
Regarding coverage of the fixtures, are we sure they are used? |
@mraspaud a couple of these fixtures are not yet used but will be in one of the next PRs. |
I was suspecting as much. Fine by me.
Yes. Sorry I thought this PR was ready. If not, make sure to check the Draft option. I almost merged this earlier. |
@djhoese also this one I guess is now ready to be merged ;) Have a nice week-end |
Lots of pre-commit failures |
@djhoese It seems that |
I will look later. |
Pytest fixtures can't be imported. They must be local (in the module) or in conftest.py somewhere
Ok. Pre-commit is happy, but we'll see if all the tests made it through my reworking. Bottom line is that pytest fixtures are magic. You can't import them. They either have to be defined in the current scope (module, class, etc) or in a special One thing to consider would be modifying the scope of these fixtures by doing https://docs.pytest.org/en/6.2.x/fixture.html#fixture-scopes |
Oh I didn't know that fixtures couldn't be imported. Good to know ... learning new stuffs everyday 😄 I try to use Any idea on what is going on? |
Ah, forget the scoping then. Although...I suppose For the error you were getting: it is saying that |
git diff origin/main **/*py | flake8 --diff
This PR
areas.py
TestBoundary
andTestGeostationaryTools
classes fromtest_area.py
into a newtest_area_boundary.py
file.TestSwathBoundary
andTestSwathBboxLonLats
classes fromtest_swath.py
into a newtest_swath_boundary.py
file.This refactor reduce the code length of such test units and facilitate the addition of new test units in the upcoming PRs.