Mesh tests by trexfeathers · Pull Request #4034 · SciTools/iris

trexfeathers · 2021-02-25T08:53:53Z

🚀 Pull Request

Description

Adds a suite of tests for the Mesh class.

Test Style

In agreement with @bjlittle (and with tentative support from @pp-mo), these tests are written in a different style that is unit-esque, but focuses on being easier to write and read. You might call them "pragmatic tests" or some-such. The basic idea:

Write tests that look more similar to how a user/developer would use the functionality in the real world.
Avoid the difficult process of abstracting every test to definitely only test one thing.
Consider re-using objects for multiple assertions in one test.
- May achieve more readable, concise testing.
- Reduces possible resource-bloat caused by the reduced abstraction.

Cons

Less granular information on failures.
So an error takes more investigation.
But not much more, and errors always need some investigation anyway.

Pros

Greatly reduced use of mock, which is known to be difficult.
Plenty of stories of mock making testing take longer than the functionality.
Can write more intuitively - faster and less difficult.
A single test can achieve the same coverage as many unit tests.
I.e. more coverage for less thinking/writing.

With this style, I was able to write the Mesh tests - ~800 lines, ~80 tests, >100 assertions - pretty much fluidly without having to stop and work out how to write a test. This definitely hasn't been true in the past when trying to achieve proper unit test abstraction.

The amount of time it takes to achieve good coverage with proper unit tests in my opinion often takes longer than the time they save when investigating errors.

This is of course only worth doing if it doesn't make the tests run excessively longer. For reference: testing the Mesh class - a fairly rich class - with reasonably good coverage takes ~100ms locally for me.

If this style causes problems / doesn't solve problems in the months ahead, I'm happy to roll back on this idea. And by all means object if you have other concerns!

Consult Iris pull request check list

* Add abstract cube summary (SciTools#3987) Co-authored-by: stephen.worsley <stephen.worsley@metoffice.gov.uk> * add nox session conda list (SciTools#3990) * Added text to state the Python version used to build the docs. (SciTools#3989) * Added text to state the Python version used to build the docs. * Added footer template that includes the Python version used to build. * added new line * Review actions * added whatsnew * Iris py38 (SciTools#3976) * support for py38 * update CI and noxfile * enforce alphabetical xml element attribute order * full tests for py38 + fix docs-tests * add whatsnew entry * update doc-strings + review actions * Alternate xml handling routine (#29) * all xml tests pass for nox tests-3.8 * restored docstrings * move sort_xml_attrs * make sort_xml_attrs a classmethod * update sort_xml_attr doc-string Co-authored-by: Bill Little <bill.james.little@gmail.com> * add jamesp to whatsnew + minor tweak Co-authored-by: James Penn <james@jamespenn.co.uk> * normalise version to implicit development release number (SciTools#3991) * Gallery: update COP maps example (SciTools#3934) * update cop maps example * comment tweaks * minor comment tweak + whatsnew * reinstate whatsnew addition * remove duplicate whatsnew * don't support mpl v1.2 (SciTools#3941) * Cubesummary tidy (SciTools#3988) * Extra tests; fix for array attributes. * Docstring for CubeSummary, and remove some unused parts. * Fix section name capitalisation, in line with existing cube summary. * Handle array differences; quote strings in extras and if 'awkward'-printing. * Ensure scalar string coord 'content' prints on one line. * update intersphinx mapping and matplotlib urls (SciTools#4003) * update intersphinx mapping and matplotlib urls * use matplotlib intersphinx where possible * review actions * review actions * update readme badges (SciTools#4004) * update readme badges * pimp twitter badge * update readme logo img src and href (SciTools#4006) * update setuptools description (SciTools#4008) Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk> Co-authored-by: stephen.worsley <stephen.worsley@metoffice.gov.uk> Co-authored-by: tkknight <2108488+tkknight@users.noreply.github.com> Co-authored-by: James Penn <james@jamespenn.co.uk> Co-authored-by: Ruth Comer <ruth.comer@metoffice.gov.uk> Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk> Co-authored-by: stephen.worsley <stephen.worsley@metoffice.gov.uk> Co-authored-by: tkknight <2108488+tkknight@users.noreply.github.com> Co-authored-by: James Penn <james@jamespenn.co.uk> Co-authored-by: Ruth Comer <ruth.comer@metoffice.gov.uk>

* MeshMetadata class. * MeshMetadata extra members for dim names. * Comment for BaseMetadata refactoring.

* add mesh coordinate manager * wip * make shape methods private + reorganise method order * review actions * partial mesh * wip

* Update mesh-data-model branch (SciTools#4009) * Add abstract cube summary (SciTools#3987) Co-authored-by: stephen.worsley <stephen.worsley@metoffice.gov.uk> * add nox session conda list (SciTools#3990) * Added text to state the Python version used to build the docs. (SciTools#3989) * Added text to state the Python version used to build the docs. * Added footer template that includes the Python version used to build. * added new line * Review actions * added whatsnew * Iris py38 (SciTools#3976) * support for py38 * update CI and noxfile * enforce alphabetical xml element attribute order * full tests for py38 + fix docs-tests * add whatsnew entry * update doc-strings + review actions * Alternate xml handling routine (#29) * all xml tests pass for nox tests-3.8 * restored docstrings * move sort_xml_attrs * make sort_xml_attrs a classmethod * update sort_xml_attr doc-string Co-authored-by: Bill Little <bill.james.little@gmail.com> * add jamesp to whatsnew + minor tweak Co-authored-by: James Penn <james@jamespenn.co.uk> * normalise version to implicit development release number (SciTools#3991) * Gallery: update COP maps example (SciTools#3934) * update cop maps example * comment tweaks * minor comment tweak + whatsnew * reinstate whatsnew addition * remove duplicate whatsnew * don't support mpl v1.2 (SciTools#3941) * Cubesummary tidy (SciTools#3988) * Extra tests; fix for array attributes. * Docstring for CubeSummary, and remove some unused parts. * Fix section name capitalisation, in line with existing cube summary. * Handle array differences; quote strings in extras and if 'awkward'-printing. * Ensure scalar string coord 'content' prints on one line. * update intersphinx mapping and matplotlib urls (SciTools#4003) * update intersphinx mapping and matplotlib urls * use matplotlib intersphinx where possible * review actions * review actions * update readme badges (SciTools#4004) * update readme badges * pimp twitter badge * update readme logo img src and href (SciTools#4006) * update setuptools description (SciTools#4008) Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk> Co-authored-by: stephen.worsley <stephen.worsley@metoffice.gov.uk> Co-authored-by: tkknight <2108488+tkknight@users.noreply.github.com> Co-authored-by: James Penn <james@jamespenn.co.uk> Co-authored-by: Ruth Comer <ruth.comer@metoffice.gov.uk> * Master to mesh data model (SciTools#4022) * Add abstract cube summary (SciTools#3987) Co-authored-by: stephen.worsley <stephen.worsley@metoffice.gov.uk> * add nox session conda list (SciTools#3990) * Added text to state the Python version used to build the docs. (SciTools#3989) * Added text to state the Python version used to build the docs. * Added footer template that includes the Python version used to build. * added new line * Review actions * added whatsnew * Iris py38 (SciTools#3976) * support for py38 * update CI and noxfile * enforce alphabetical xml element attribute order * full tests for py38 + fix docs-tests * add whatsnew entry * update doc-strings + review actions * Alternate xml handling routine (#29) * all xml tests pass for nox tests-3.8 * restored docstrings * move sort_xml_attrs * make sort_xml_attrs a classmethod * update sort_xml_attr doc-string Co-authored-by: Bill Little <bill.james.little@gmail.com> * add jamesp to whatsnew + minor tweak Co-authored-by: James Penn <james@jamespenn.co.uk> * normalise version to implicit development release number (SciTools#3991) * Gallery: update COP maps example (SciTools#3934) * update cop maps example * comment tweaks * minor comment tweak + whatsnew * reinstate whatsnew addition * remove duplicate whatsnew * don't support mpl v1.2 (SciTools#3941) * Cubesummary tidy (SciTools#3988) * Extra tests; fix for array attributes. * Docstring for CubeSummary, and remove some unused parts. * Fix section name capitalisation, in line with existing cube summary. * Handle array differences; quote strings in extras and if 'awkward'-printing. * Ensure scalar string coord 'content' prints on one line. * update intersphinx mapping and matplotlib urls (SciTools#4003) * update intersphinx mapping and matplotlib urls * use matplotlib intersphinx where possible * review actions * review actions * update readme badges (SciTools#4004) * update readme badges * pimp twitter badge * update readme logo img src and href (SciTools#4006) * update setuptools description (SciTools#4008) * cirrus-ci compute credits (SciTools#4007) * update release process (SciTools#4010) * Stop using deprecated aliases of builtin types (SciTools#3997) * Stopped using deprecated aliases of builtin types. This is required to avoid warnings starting with NumPy 1.20.0. * Update lib/iris/tests/test_cell.py Co-authored-by: Bill Little <bill.little@metoffice.gov.uk> * Update lib/iris/tests/test_cell.py Co-authored-by: Bill Little <bill.little@metoffice.gov.uk> * Updated whatsnew. Co-authored-by: Bill Little <bill.little@metoffice.gov.uk> * celebrate first time iris contributors (SciTools#4013) * Docs unreleased banner (SciTools#3999) * baseline * removed debug comments * reverted * remove line * Testing * testing extensions * testing rtd_version * fixed if * removed line * tidy up * tidy comments * debug of pre-existing rtd variables * added reminder * testing * testing still * updated comments * added whatsnew * expanded the if conditiion * review actions * Update layout.html Remove alternative banner that used the RestructuredText notation. * review actions * drop __unicode__ method usage (SciTools#4018) * cirrus-ci conditional tasks (SciTools#4019) * cirrus-ci conditional tasks * use bc for bash arithmetic * revert back to sed * use expr * reword * minor documentation changes * review actions * make iris.common.metadata._hexdigest public (SciTools#4020) Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk> Co-authored-by: stephen.worsley <stephen.worsley@metoffice.gov.uk> Co-authored-by: tkknight <2108488+tkknight@users.noreply.github.com> Co-authored-by: James Penn <james@jamespenn.co.uk> Co-authored-by: Ruth Comer <ruth.comer@metoffice.gov.uk> Co-authored-by: Alexander Kuhn-Regnier <ahf.kuhnregnier@gmail.com> Co-authored-by: Patrick Peglar <patrick.peglar@metoffice.gov.uk> Co-authored-by: stephen.worsley <stephen.worsley@metoffice.gov.uk> Co-authored-by: tkknight <2108488+tkknight@users.noreply.github.com> Co-authored-by: James Penn <james@jamespenn.co.uk> Co-authored-by: Ruth Comer <ruth.comer@metoffice.gov.uk> Co-authored-by: Alexander Kuhn-Regnier <ahf.kuhnregnier@gmail.com>

* ConnectivityManager first pass. * ConnectivityManager align with proposed CoordManager. * Connectivity Manager review actions. * Connectivity Manager more review changes. * Use metadata_manager for Mesh location dimension. * Mesh dimension name abstraction. * Align Cooord and Connectivity Managers filters methods. * Completed Mesh class. * filter_cf improvements. * Moved filter_cf. * Mesh connectivity manager namedtuples comment. * Mesh removed trailing underscores. * Mesh _set_dimension_names improvements. * Mesh import rationalisation. * Mesh connectivity manager remove NDIM. * Connectivity manager use lazy indices_by_src(). * Connectivity manager clearer removal syntax. * Connectivity manager don't override __init__. * Connectivity manager correct base class syntax. * Metadata filter hexdigest reference fix. * test_MeshMetadata fix. * Rename filter to metadata_filter.

* minor fixes * wip

…ss_mergeback

trexfeathers · 2021-02-25T09:19:51Z

Something jazzy going on with the tests that I don't think can be related to my work. I'm trying running the whole suite locally to see what's up...

… to play nicely with unittest.

bjlittle

@trexfeathers Great stuff!

Push the outcome of our offline conversation and I'll re-review again 👍

lib/iris/experimental/ugrid.py

bjlittle · 2021-02-25T13:09:22Z

lib/iris/experimental/ugrid.py

+            node = edge = face = False
+        elif true_count == 0:
+            # Treat None as True in this case.
+            node, edge, face = [True if arg is None else arg for arg in args]


@trexfeathers I kinda disagree with this change in behaviour.

What was there before was richer, where as what you're proposing now is quite restrictive and you're forcing the user to be explicit. Happy to chat about this to clarify.

bjlittle

Awesome thanks 👍

bjlittle and others added 19 commits February 12, 2021 16:16

add ugrid mesh-api stubs (SciTools#4001)

1f918ef

add additional mesh stubs (SciTools#4005)

36a93b0

MeshMetadata class. (SciTools#4002)

41a72f9

* MeshMetadata class. * MeshMetadata extra members for dim names. * Comment for BaseMetadata refactoring.

add meshmetadata services (SciTools#4012)

30f9220

Mesh api coord manager (SciTools#4015)

682136b

* add mesh coordinate manager * wip * make shape methods private + reorganise method order * review actions * partial mesh * wip

minor fixes (SciTools#4025)

f4a8108

* minor fixes * wip

add mesh pickle support (SciTools#4026)

ee19869

Merge remote-tracking branch 'upstream/ng-vat-mesh-api' into mesh_cla…

8f02778

…ss_mergeback

Test Mesh WIP.

ef4c411

Mesh face_dimension not set for topology_dimension=1.

597bb1a

Mesh testing WIP.

02f991b

Mesh tests WIP.

9c2d9ef

Mesh tests WIP.

583ae4a

Mesh tests complete.

a1cb54d

Apply mesh_tests changes onto mesh-data-model.

534d899

Mesh repr tests.

9217a59

trexfeathers added Feature: UGRID Type: Testing Type: Feature Branch Highlight this for a feature branch labels Feb 25, 2021

trexfeathers requested a review from bjlittle February 25, 2021 08:53

trexfeathers assigned bjlittle Feb 25, 2021

experimental.ugrid restore class ordering.

efdf79a

trexfeathers added 2 commits February 25, 2021 11:31

Mesh tests - move global and class variables into setUpClass methods,…

6837a6b

… to play nicely with unittest.

Delete commented code.

447833d

bjlittle requested changes Feb 25, 2021

View reviewed changes

Mesh clearer distinction between coords and connectivities filters.

acdf188

Mesh tests slight readability improvement.

d70ad7a

bjlittle merged commit 48028e9 into SciTools:mesh-data-model Feb 25, 2021

bjlittle reviewed Feb 25, 2021

View reviewed changes

trexfeathers deleted the mesh_tests branch August 31, 2021 15:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mesh tests#4034

Mesh tests#4034
bjlittle merged 24 commits intoSciTools:mesh-data-modelfrom
trexfeathers:mesh_tests

trexfeathers commented Feb 25, 2021 •

edited

Loading

Uh oh!

trexfeathers commented Feb 25, 2021

Uh oh!

bjlittle left a comment

Uh oh!

Uh oh!

bjlittle Feb 25, 2021

Uh oh!

trexfeathers Feb 25, 2021

Uh oh!

bjlittle left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

trexfeathers commented Feb 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚀 Pull Request

Description

Test Style

Uh oh!

trexfeathers commented Feb 25, 2021

Uh oh!

bjlittle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bjlittle Feb 25, 2021

Choose a reason for hiding this comment

Uh oh!

trexfeathers Feb 25, 2021

Choose a reason for hiding this comment

Uh oh!

bjlittle left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

trexfeathers commented Feb 25, 2021 •

edited

Loading