-
Notifications
You must be signed in to change notification settings - Fork 281
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
Added a draft style guide for iris pytest #5785
base: FEATURE_pytest_conversion
Are you sure you want to change the base?
Added a draft style guide for iris pytest #5785
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## FEATURE_pytest_conversion #5785 +/- ##
=============================================================
+ Coverage 89.71% 89.74% +0.03%
=============================================================
Files 90 92 +2
Lines 22816 22940 +124
Branches 5438 5462 +24
=============================================================
+ Hits 20469 20588 +119
- Misses 1617 1620 +3
- Partials 730 732 +2 ☔ 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.
Thanks for this @ESadek-MO!
You've definitely captured everything the team discussed offline, and the spirit of compromise is much more realistic than many style guides I've read in the past 💐
Some changes below
============= | ||
Testing tools | ||
============= |
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.
This heading has not rendered at the correct level
------------- | ||
|
||
As a package capable of generating graphical outputs, Iris has utilities for | ||
creating and updating graphical tests - see :ref:`testing.graphics` for more |
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've read through that dedicated page and I don't believe anything needs changing 👍
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.
Can confirm that this is a faithful lift-and-shift from all the 'sub-pages'. The sequence makes sense to me.
Happy to delete those now?
Context managers | ||
---------------- |
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.
This heading needs to be a higher level
Directory | ||
--------- |
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.
This section overlaps significantly with:
iris/docs/src/developers_guide/contributing_tests.rst
Lines 11 to 12 in 37a342f
Test Categories | |
=============== |
After the change from ``unittest`` to ``pytest``, ``IrisTest.patch`` has been | ||
converted into :meth:`~iris.tests._shared_utils.patch`. | ||
|
||
This is currently not implemented, and will raise an error if called. |
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 believe we have discovered the mock.patch
pattern is the perfect replacement for this?
:meth:`~iris.tests.IrisTest_nometa.assert_no_warnings_regexp` and | ||
:meth:`~iris.tests.IrisTest_nometa.assert_logs`. |
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.
Needs changing to reference _shared_utils
assertions, such as :meth:`~iris.tests._shared_utils.assert_array_equal`, and | ||
:meth:`~iris.tests._shared_utils.assert_array_almost_equal`. |
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.
These are now :func:
rather than :meth:
:meth:`~iris.tests._shared_utils.assert_CML_approx_data` | ||
:meth:`~iris.tests._shared_utils.assert_CDL` | ||
:meth:`~iris.tests._shared_utils.assert_CML` and | ||
:meth:`~iris.tests._shared_utils.assert_text_file`. See docstrings for more |
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.
:func:
|
||
All functions listed on this page are defined within | ||
:mod:`iris.tests._shared_utils`. They can be accessed within a test using | ||
``_shared_utils.exampleFunction``. |
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.
Probably best to use the non-unittest format e.g. example_function
🚀 Pull Request
This is a style guide to offer some consistency on iris' approach to testing.
Formatting and phrasing is not final, at the moment stands as a PoC.