iris-grib icon indicating copy to clipboard operation
iris-grib copied to clipboard

Move iris-grib to pytest

Open jamesp opened this issue 4 years ago • 5 comments

### Tasks
- [ ] https://github.com/SciTools/iris-grib/issues/412

jamesp avatar Mar 01 '21 12:03 jamesp

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?

TomDufall avatar Jun 04 '21 10:06 TomDufall

@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

pp-mo avatar Jun 04 '21 11:06 pp-mo

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!

trexfeathers avatar Jun 04 '21 12:06 trexfeathers

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.

TomDufall avatar Jun 09 '21 22:06 TomDufall

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

trexfeathers avatar Sep 06 '24 16:09 trexfeathers