Skip to content

Rewrite result_path() for pytest#5817

Merged
ESadek-MO merged 2 commits into
SciTools:FEATURE_pytest_conversionfrom
trexfeathers:result_path
Mar 12, 2024
Merged

Rewrite result_path() for pytest#5817
ESadek-MO merged 2 commits into
SciTools:FEATURE_pytest_conversionfrom
trexfeathers:result_path

Conversation

@trexfeathers

@trexfeathers trexfeathers commented Mar 11, 2024

Copy link
Copy Markdown
Contributor

No description provided.

@trexfeathers trexfeathers requested a review from ESadek-MO March 11, 2024 18:04
@github-actions github-actions Bot added the benchmark_this Request that this pull request be benchmarked to check if it introduces performance shifts label Mar 11, 2024
@trexfeathers trexfeathers changed the base branch from main to FEATURE_pytest_conversion March 11, 2024 18:04
@codecov

codecov Bot commented Mar 11, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (FEATURE_pytest_conversion@1ca9400). Click here to learn what that means.

Additional details and impacted files 
@@                     Coverage Diff                      @@
##             FEATURE_pytest_conversion    #5817   +/-   ##
============================================================
  Coverage                             ?   89.74%           
============================================================
  Files                                ?       92           
  Lines                                ?    22940           
  Branches                             ?     5462           
============================================================
  Hits                                 ?    20588           
  Misses                               ?     1620           
  Partials                             ?      732

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ESadek-MO
ESadek-MO requested changes Mar 12, 2024
View reviewed changes

@ESadek-MO ESadek-MO left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @trexfeathers! Just one change for you, otherwise LGTM!

Comment thread lib/iris/tests/_shared_utils.py
output_func = basename
else:
output_func = caller_func.replace("test_", "")
output_path = output_path / output_func

@ESadek-MO ESadek-MO Mar 12, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

if caller_func == "<module>":
    output_path = output_path / output_func

@trexfeathers trexfeathers Mar 12, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iris/lib/iris/tests/_shared_utils.py

Lines 248 to 255 in e999c78

# Generate the file name from the calling function name.
if basename is not None:
output_func = basename
elif caller_func == "<module>":
output_func = ""
else:
output_func = caller_func.replace("test_", "")
output_path = output_path / output_func

I tested and doing Path("foo/bar") / "" returns foo/bar with no additions, so this should work fine.

@trexfeathers

trexfeathers commented Mar 12, 2024

Copy link
Copy Markdown
Contributor Author

The benchmark results here are misleading. Benchmarks were only triggered because I originally targetted main, and a change in lockfiles was detected. Once I targetted FEATURE_pytest_conversion there was no detected change in lockfiles and no benchmarks ran.

The performance regressions are definitely associated with lockfile changes as you can also see them in #5791 and #5812.

I'm deleting those comments.

@SciTools SciTools deleted a comment from github-actions Bot Mar 12, 2024
@SciTools SciTools deleted a comment from github-actions Bot Mar 12, 2024
ESadek-MO
ESadek-MO approved these changes Mar 12, 2024
View reviewed changes

@ESadek-MO ESadek-MO left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, will merge once tests pass Gw!

@trexfeathers trexfeathers added Type: Feature Branch Highlight this for a feature branch and removed benchmark_this Request that this pull request be benchmarked to check if it introduces performance shifts labels Mar 12, 2024
@ESadek-MO ESadek-MO merged commit 9f71331 into SciTools:FEATURE_pytest_conversion Mar 12, 2024
@trexfeathers trexfeathers deleted the result_path branch May 3, 2024 15:25
trexfeathers added a commit that referenced this pull request Nov 1, 2024
@ESadek-MO @bjlittle
@web-flow @stephenworsley @pp-mo @trexfeathers @pre-commit-ci @HGWright
Merge pytesting contributions to main (#6207)
7bca716 
* 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 <patrick.peglar@metoffice.gov.uk>

* Update lib/iris/tests/unit/common/metadata/test_CellMeasureMetadata.py

Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk>

* Update lib/iris/tests/unit/common/metadata/test_CellMeasureMetadata.py

Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk>

* Update lib/iris/tests/unit/common/metadata/test_CellMeasureMetadata.py

Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk>

* review actions

---------

Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk>

* 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 <patrick.peglar@metoffice.gov.uk>

---------

Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk>

* 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 <40734014+trexfeathers@users.noreply.github.com>

* added whatsnew entries (#6211)

* added whatsnew entries

* corrected _ to - in githubname

* whatsnew corrections

---------

Co-authored-by: Martin Yeo <40734014+trexfeathers@users.noreply.github.com>

---------

Co-authored-by: Bill Little <bill.james.little@gmail.com>
Co-authored-by: Lockfile bot <noreply@github.com>
Co-authored-by: stephenworsley <49274989+stephenworsley@users.noreply.github.com>
Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk>
Co-authored-by: Martin Yeo <40734014+trexfeathers@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Wright <henrywright@sky.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ESadek-MO ESadek-MO ESadek-MO approved these changes

Assignees

No one assigned

Labels

Feature: pytest Type: Feature Branch Highlight this for a feature branch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@trexfeathers @ESadek-MO