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

Move iris-grib to pytest #253

Open
1 task done
jamesp opened this issue Mar 1, 2021 · 5 comments
Open
1 task done

Move iris-grib to pytest #253

jamesp opened this issue Mar 1, 2021 · 5 comments

Comments

@jamesp
Copy link
Member

jamesp commented Mar 1, 2021

Tasks

  1. bjlittle
@TomDufall
Copy link
Contributor

I'm thinking of messing about with this a bit, hopefully within a few weeks. Are there any particular objectives here beyond standardisation and all the good stuff Pytest provides? I.e. are there any specific pain points this is trying to resolve and that may need extra focus?

@pp-mo
Copy link
Member

pp-mo commented Jun 4, 2021

@TomDufall thinking of messing about with this a bit

Well, good for you ! 💐 Really want to encourage this.
We've been thinking about it for a while, more in the Iris space than iris-grib, but actually iris-grib could make a nice (smaller) starting-point.

There's a question as to adopting PyTest as an engine, as against rewriting unittest-style code for PyTest.
You probably don't need to rewrite anything (much), but there are obvious benefits waiting there.

A braindump of some points I've noted so far...

key want : get rid of "nose" which is rather ancient and unsupported

  • we've hung onto this mostly for the parallel-testing benefit
    • as for Iris
    • without it, CI testing timeouts could be a real problem
  • probably want to replace with pytest-xdist, or possibly pytest-parallel (process or ?threading)

benefits

  • enabler : it would be great to start using code coverage
  • common usage : now very popular
  • ease : PyTest-specific code is much more concise
  • parametrisation : pure-unittest makes writing "matrix-ed" tests hard, but PyTest does this well
  • better support for : temporary files, warnings-testing, errors-testing

possible reasons to not do it

  • clever ways of interpreting the python testcode makes the mechanics implicit, not explicit [non-Pythonic!]
    so basically new syntax needs to be learnt -- though unittest is arguably also guilty of this (=discovery; inheritance oddities)
  • we avoided dependending on "nose" when that seemed hot -- and now we're glad we did !!

drawbacks

  • there's a hint (some people seem to think) that Mock doesn't mix well with PyTest -- though I'm not sure why
  • the Pytest 'plain assert' approach is not so great for complex "types of equality testing"
    • e.g. see the multiple specialised array-comparison routines in Iris.tests.IrisTest
    • likewise the Iris assertCDL, and _check_same routines, which are much used + provide some very useful automation
    • think we may need to replace these with something like
      assert my_special_eqtest_failedbecause(result, expect) == ""
  • exisiting inheritance usage of 'mixins' may be complicated to rewrite

@trexfeathers
Copy link
Contributor

A braindump of some points I've noted so far...

@pp-mo that's really helpful to see - I kept thinking I ought to be taking notes when you were discussing this with me!

@TomDufall
Copy link
Contributor

Good news - replacing the test command in nox with python -m pytest . appears to run everything just fine - pytest has pretty good compatibility with unittest, but I was still a bit nervous. It ran 529 tests, which is the same as the largest number of tests showing in CI currently - presumably this is for 'default' tests, although the concurrent logging makes this not very clear. Incidentally, given 'default' also runs unit and integration tests, I think most tests are being run twice at the moment.

A single session (python3.7, iris_source=conda-forge) took up 16s of pytest time on my box (i.e. + env setup) before I've even tried pytest-xdist. This was just a quick compatibility check to see how much work there was to do - I haven't yet started on duplicating any existing arguments or features that we've got with the current test setup.

Essentially, this should mean that a pretty minimal migration to pytest is possible and the move to native pytest (plain assertions, remove main blocks, remove small classes, use fixtures, etc) can happen later as prioritisation/effort/interest dictates.

@trexfeathers
Copy link
Contributor

I believe this needs to stay open - we switched the runner in #420 but the need to rewrite into PyTest format remains.

@trexfeathers trexfeathers reopened this Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

5 participants