-
Notifications
You must be signed in to change notification settings - Fork 284
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
pytest migration for unit.common.lenient.test__Lenient #5828
pytest migration for unit.common.lenient.test__Lenient #5828
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## FEATURE_pytest_conversion #5828 +/- ##
=============================================================
- Coverage 89.74% 89.74% -0.01%
=============================================================
Files 92 92
Lines 22942 22940 -2
Branches 5464 5462 -2
=============================================================
- Hits 20590 20588 -2
Misses 1620 1620
Partials 732 732 ☔ View full report in Codecov by Sentry. |
03647f5
to
668e1d3
Compare
ab724f6
to
051fa3e
Compare
self.client = "myclient" | ||
self.lenient = _Lenient() | ||
|
||
def test_missing_service_str(self): | ||
self.assertFalse(self.lenient("myservice")) | ||
assert bool(self.lenient("myservice")) is False |
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.
IIUC,
assert bool(x) is True
/ assert bool(x) is False
are completely equivalent to
assert x
/ assert not x
.
So... I'm thinking those would actually be simpler + easier to read.
But I guess you disagree !?!
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.
Possibly a bit of a rant going here, but here's my thinking ..
I believe that "truthiness" in python is just always whatever "bool(x)" returns, and what the "if" and "assert" statements consider true or false is precisely "whatever bool says".
So you shouldn't write "if bool(x)" or "assert bool(x)" because that is already a guaranteed part of what "if" and "assert" are defined to do.
And if "if bool(x)" is redundant, because that's exactly what "if" does already, then also "if bool(x) is True" is doubly redundant, because that's exactly what "bool" does.
As far as I understand it, you can never have an x which passes "x ^ bool(x)", or "bool(x) & bool(x) is not True" or "not bool(x) & bool(x) is not False". Because "bool" is the only decider of "truthiness" in Python, and it only ever returns True or False.
And I think that whatever goes for "if" here goes for "assert" just as well.
I looked up "assert" in the Python docs, and they do state that it is equivalent to "if not x: raise"
assert context["active"], expected["active"] | ||
assert set(context["client"]), set(expected["client"]) | ||
assert post, self.default |
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.
assertEqual translations gone wrong here ...
assert context["active"], expected["active"] | |
assert set(context["client"]), set(expected["client"]) | |
assert post, self.default | |
assert context["active"] == expected["active"] | |
assert set(context["client"]) == set(expected["client"]) | |
assert post == self.default |
assert service not in lenient.__dict__ | ||
lenient.register_service(service) | ||
assert service in lenient.__dict__ | ||
assert bool(isinstance(lenient.__dict__[service], Iterable)) is False |
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.
Even if the bool(lenient[x]) is False/True
style has value in asserting the behaviour of lenient[x] treated as a boolean,
I really don't think it has value here, because we already totally know that isinstance always returns a bool.
So,
assert bool(isinstance(lenient.__dict__[service], Iterable)) is False | |
assert not isinstance(lenient.__dict__[service], Iterable) |
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.
NB there are a few more of these ones, too
Cool, that is a big one ! |
* Add pytest-mock dependency (#5811) * Updated environment lockfiles * added pytest iris class (#5808) * added pytest iris class * actioned majority of review comments * converted remaining functions to camel_case * actioned majority of review comments * make unit/config pytest (#5810) * Convert all tests to pytest. * Don't use staticmethod on fixtures. * Everything in 'TestConcatenate__dask' can use the same sample cubes. * Spurious parentheses in 'pytest.fixture()' * Convert all tests to pytest. * Rewrite `result_path()` for pytest (#5817) * Rewrite result_path. * Review comments. * Configure pytest-mock as pytest required-plugin (#5815) * pytest migration ruff PT compliance for unit.concatenate (#5823) * pytest migration for unit.common.test_Lenient (#5822) * pytest migration for unit.common.lenient.test_Lenient * assert order * review actions * Convert to pytest. * Better implementation of `_shared_utils.results_path()` - using PyTest `request` (#5827) * Rewrite result_path to use the pytest request fixture. * More explicit guidance on the request fixture. * Convert to pytest. * Convert to pytest. * Convert graphics testing conveniences to PyTest (#5832) * PyTest-compatible check_graphic. * Demonstrate new check_graphic in test_plot. * 7c4f700 * Revert "7c4f7003a" This reverts commit 68f81ac. * Revert "Demonstrate new check_graphic in test_plot." This reverts commit 7c4f700. * Tidy up other unittest references in iris.tests.graphics. * Tidy up other unittest references in iris.tests.graphics. * check_graphic_caller docstring example. * Make check_graphic_caller only accessible from conftest. * Simplify data fixtures. * Simplify data fixtures. * Simplify data fixtures. * Simplify Mock usage. * Tidy equality checks. * Convert to pytest, * pytest migration for unit.common.lenient.test__lenient_client (#5842) * pytest migration for unit.common.lenient.test__lenient_client * review actions * pytest migration for unit.common.lenient.test__Lenient (#5828) * pytest migration for unit.common.lenient.test__Lenient * ruff PT rule compliant * fixes * refactor * assert fixes * review actions * Convert to pytest. * Convert `tests/test_plot` to PyTest (#5839) * Convert test_plot to PyTest. * Add explanatory comments. * Make ruff PT compliant. * Convert test_quickplot. * Test aggregate pytest (#5846) * pytestify test_aggregate_by. * pytestify test_lazy_aggregate_by. * pytest migration for unit.common.lenient.test__lenient_service (#5843) * pytest migration for unit.common.mixin.test__get_valid_standard_name (#5847) * pytest migration for unit.common.mixin.test_CFVariableMixin (#5849) * pytest migration for unit.common.lenient.test__qualname (#5845) * pytest migration for unit.common.lenient.test__qualname * use setup_method * use setup fixture * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * pytest migration for unit.common.mixin.test_LimitedAttributeDict (#5851) * pytest migration for unit.common.mixin.test_LimitedAttributeDict * use setup_method * use setup fixture * pytest migration for unit.common.metadata.test_NamedTupleMeta (#5852) * Make test_analysis PyTest (#5859) * Fixes to _shared_utils. * Make test_analysis.py pytest. * Make test_analysis_calculus.py pytest. * Remove np.testing. * Convert to pytest. * Convert unit/fileformats/__init__ code to pytest + move to pp_load_rules where it belongs. * Convert unit/fileformats/pp_load_rules to pytest. * converted cube directory tests to pytest (#5837) * converted asserts to pytest * pytestified and search replaced. Ruffed * resolved old references * refactored preexisting setup_methods into _setup * further changes * fixed mock and CML assert * fixed cdl call in shared_utils * all tests passing * review changes * tests passing * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Convert tests in unit/util to pytest (#5853) * pytestify, and test__coord_regular * search and replace * ruff * fixed most failures * renamed result dir * fixed review comments * whoops, incorrectly called assert_original_metadata with request after removing it * corrected assert * Use mocker.spy in place of mock.patch. * pytest migration for unit.common.metadata.test_AncillaryVariableMetadata (#5854) * pytest migration for unit.common.metadata.test_AncillaryVariableMetadata * use setup fixture * review actions * Convert `unit.plot` and `.quickplot` to PyTest (#5866) * Convert tests.unit.plot to PyTest. * Convert tests.unit.quickplot to PyTest. * Convert setups to autouse fixtures. * Convert to Pytest. * Convert to Pytest. * Convert to pytest. * Convert to pytest, move test_rules into /rules. * Convert to pytest * Convert remaining graphics tests to PyTest (#5867) * Convert test_mapping to PyTest. * Convert integration.plot to PyTest. * Convert unit/fileformats/structured_array_identification to pytest. * Review changes. * pytest migration for unit.common.metadata.test_BaseMetadata (#5868) * pytest migration for unit.common.metadata.test_BaseMetadata * review actions * pytest migration of unit.common.metadata.test_hexdigest (#5874) * pytest migration of unit.common.metadata.test_metadata_filter (#5875) * pytest migration for unit.common.metadata.test_CubeMetadata (#5881) * pytest migration of unit.common.metadata.test_metadata_manager_factory (#5876) * pytest migration of unit.common.metadata.test_metadata_manager_factory * review actions * More root pytest (#5883) * Convert root test_util to PyTest. * Convert root test_pp to PyTest. * Review comments - make tests easier to understand. * pytest migration of unit.common.metadata.test_CellMeasureMetadata (#5878) * pytest migration of unit.common.metadata.test_CellMeasureMetadata * Update lib/iris/tests/unit/common/metadata/test_CellMeasureMetadata.py Co-authored-by: Patrick Peglar <[email protected]> * Update lib/iris/tests/unit/common/metadata/test_CellMeasureMetadata.py Co-authored-by: Patrick Peglar <[email protected]> * Update lib/iris/tests/unit/common/metadata/test_CellMeasureMetadata.py Co-authored-by: Patrick Peglar <[email protected]> * Update lib/iris/tests/unit/common/metadata/test_CellMeasureMetadata.py Co-authored-by: Patrick Peglar <[email protected]> * review actions --------- Co-authored-by: Patrick Peglar <[email protected]> * pytest migration of unit.common.metadata.test_CoordMetadata (#5880) * pytest migration of unit.common.metadata.test_CoordMetadata * review actions * Update lib/iris/tests/unit/common/metadata/test_CoordMetadata.py Co-authored-by: Patrick Peglar <[email protected]> --------- Co-authored-by: Patrick Peglar <[email protected]> * pytest migration of unit.common.resolve.test_Resolve (#5882) * pytest migration of unit.common.resolve.test_Resolve * review actions * regen lockfiles * precommit fixes * fix some test failures * Added a style guide for iris pytest (#5785) * Added a draft style guide for iris pytest * most review comments * refactored documentation * fixed doclinks * reslolved review comments * removed excess pages * conversion checklist * pre-lunch changes * majority review requests, rough reshuffle of Test Categories * further reshuffle of Test Categories * review stuffies * fixed a coup of review comments * fixed a doctest failures * reworded function and class intros --------- Co-authored-by: Martin Yeo <[email protected]> * added whatsnew entries (#6211) * added whatsnew entries * corrected _ to - in githubname * whatsnew corrections --------- Co-authored-by: Martin Yeo <[email protected]> --------- Co-authored-by: Bill Little <[email protected]> Co-authored-by: Lockfile bot <[email protected]> Co-authored-by: stephenworsley <[email protected]> Co-authored-by: Patrick Peglar <[email protected]> Co-authored-by: Martin Yeo <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Henry Wright <[email protected]>
🚀 Pull Request
Description
Consult Iris pull request check list
Add any of the below labels to trigger actions on this PR: