Skip to content
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

Remove test timings #5101

Merged
merged 4 commits into from
Dec 14, 2022
Merged

Remove test timings #5101

merged 4 commits into from
Dec 14, 2022

Conversation

rcomer
Copy link
Member

@rcomer rcomer commented Dec 9, 2022

🚀 Pull Request

Description

Here is some more Iris functionality that I did not know existed, until @pp-mo mentioned it the other day. Described by the removed comment

# An environment variable controls whether test timings are output.
#
# NOTE: to run tests with timing output, nosetests cannot be used.
# At present, that includes not using "python setup.py test"
# The typically best way is like this :
# $ export IRIS_TEST_TIMINGS=1
# $ python -m unittest discover -s iris.tests

I tried to run this with unittest discover as the comment recommends, but got errors, particularly from test_pandas.py. I found that this functionality can be used with pytest -s -v, so if we do want to keep the functionality we should probably document that. I'm unsure if it can be made to work with pytest-xdist.

In any case, pytest provides its own functionality to tell you the timings of the slowest running tests via the --durations flag, and of course we now have some pytest tests which don't inherit from IrisTest, so won't work with the bespoke functionality unless we go through and add the decorator.

More generally, the more we can simplify these test classes, the better chance we have of eventually ditching unittest altogether.

This functionality was added at #2372.


Consult Iris pull request check list

Copy link
Member

@lbdreyer lbdreyer left a comment

Choose a reason for hiding this comment

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

Looking great! Thanks for doing this @rcomer !

I spotted one other mention of IrisTest_nometa here: https://github.com/SciTools/iris/blame/main/lib/iris/tests/graphics/README.md#L27

Do you mind also updating that in this PR?

@rcomer
Copy link
Member Author

rcomer commented Dec 14, 2022

Thanks @lbdreyer, good spot! I only grepped the python files. Now fixed and also rebased to address the whatsnew conflict. I don't know why the CI isn't running 😕 [close and reopen fixed that]

@rcomer rcomer closed this Dec 14, 2022
@rcomer rcomer reopened this Dec 14, 2022
@lbdreyer lbdreyer merged commit 5555d45 into SciTools:main Dec 14, 2022
@rcomer rcomer deleted the remove-test-timings branch December 14, 2022 17:41
tkknight added a commit to tkknight/iris that referenced this pull request Jan 5, 2023
* upstream/main: (167 commits)
  pip pin for sphinx<5 (SciTools#5122)
  [pre-commit.ci] pre-commit autoupdate (SciTools#5120)
  Bump actions/stale from 6 to 7 (SciTools#5117)
  [pre-commit.ci] pre-commit autoupdate (SciTools#5114)
  Correct heading for v3.4 release highlights. (SciTools#5110)
  Announce @ESadek-MO as a core Iris developer. (SciTools#5111)
  Remove test timings (SciTools#5101)
  Switch order of options and parameter in `ncgen` command (SciTools#5105)
  [pre-commit.ci] pre-commit autoupdate (SciTools#5107)
  Updated environment lockfiles (SciTools#5104)
  DOC: improve gallery test instructions (SciTools#5100)
  Updated environment lockfiles (SciTools#5092)
  Update What's New for 3.4 release. (SciTools#5088)
  [pre-commit.ci] pre-commit autoupdate (SciTools#5086)
  Updated environment lockfiles (SciTools#5085)
  Updated environment lockfiles (SciTools#5080)
  Restore latest What's New files.
  Documentation updates for `v3.4.0rc0` release. (SciTools#5078)
  What's New fixes. (SciTools#5077)
  More accurate netcdf4 pin `<1.6.1`. (SciTools#5075)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants