Various testing fixes

[pp-mo]

Replacing #282
As I needed a new branch

[pp-mo]

Note: something truly crazy was happening before this ...

• we were running the test runner
  as python -m iris_grib.tests.runner --default-tests --unit-tests --integration-tests
• .. but the "--default-tests" option actually includes the other two, due to the test options discovery usage
• and in fact, it looks like all of this is happening twice (?but still not sure how?), so that every test is being run not twice but 4 times
  • checkout the most recent successful test runs

[pp-mo]

it looks like all of this is happening twice

At present the noxfile supports a dual test-run matrix for Iris versions : one from 'latest' (repo branch 'main') and one from 'conda-forge' (i.e. latest released version).
That accounts for the "extra x2"

Basically, this was causing trouble because a non-bugfix change was merged into Iris v3.0.x.
( Oops, guess who did that ... )

I will investigate configuring cirrus to do separate tasks for the 'latest' and 'conda-forge' testing
(when I got this working?)

[pp-mo]

I would have retained that test, but it seems to be hitting an unrelated error, I'm guessing related to ecCodes, so I think we can drop that rather than fix it.

It was there to ensure that it was "safe" to fetch a zero-length slice (because Dask appeared to need it).
But we since decided that we don't need to support that.

[pp-mo]

OK, although it is explicitly possible to use "matrix" more than once, I think that does require different Cirrus keywords to apply it to. Whereas our options are both implemented with the 'env' key.

So, the previous commit
expands out the 2*2 options (2 for iris-source, and 2 for python-version).

Meanwhile, this latest commit, does allow me to specify "python-version" and "iris-source" separately.
But to do that, I used the nasty kludge of specifying both the two different allowed names of the 'env' key. 🤮

Presumably, another way is to use an "incorrect" YAML syntax with a repeated key, as we had before for the python versions.
What do you think to this @trexfeathers ?

[pp-mo]

YAML syntax with a repeated key

( Sorry for the braindump :
investigating this, I got in a bit deep + I thought I'd just record what I found here, in case of future use.
)

It appears that the ordering + uniqueness of "mapping keys" is not a given in many implementations of YAML
(search "yaml duplicate keys"), though AFAICT it was always supposed to be there in the spec.
Personally I am persuaded that YAML content "ought" to be equivalent to the expected Python objects, and therefore repeated and duplicated keys, or relying on the ordering of keys, are really not safe.
( Aside: I had thought that a desired equivalence to JSON would seal it, but that has a similar problem. What a mess, eh ? )

I think Cirrus may be using its own fork of a popular YAML engine -- though I'm not entirely sure why, and they may possibly have reverted to the original by now?.
Anyway, that scheme I think provides an iteration syntax over mappings, though importantly it sounds like they are trying to get rid of that: tidy up for version-3.

So in short (!) I think you were right @trexfeathers that Cirrus is not strict about YAML, but ideally I think we should be -- at least to be future-proof.

[trexfeathers]

@pp-mo out of the two options you present I definitely prefer 8762cf5, although I would certainly like to use 'true' matrix construction if it were possible to write in a non-confusing way using correct syntax.

Intuitively I would have hoped the below would be valid:

env:
  matrix:
    PY_VER:
      - 3.7
      - 3.8
    IRIS_SOURCE:
      - "conda-forge"
      - "source"

[pp-mo]

Intuitively I would have hoped the below would be valid ...

That's an interesting approach.

I wonder if I have been over-concerned about the 'special' nature of the 'env' construct?

I think, from closer inspection of the docs, that 'matrix' must work in a pre-processing kind of way, like a macro applying to the source text, which is replaced with one of its options wherever it occurs.
That does seem to be implied by their 'complex' example:

task:
  container:
    matrix:
      - image: node:latest
      - image: node:lts
  node_modules_cache:
    folder: node_modules
    fingerprint_script:
      - node --version
      - cat yarn.lock
    populate_script: yarn install
  matrix:
    - name: Build
      build_script: yarn build
    - name: Test
      test_script: yarn run test

So in this, each 'branch' of the (last) matrix is providing more than one key in the containing definition ...

If it does works like that, then the 'env' construct would probably not behave differently from anything else.
In which case I think the correct way could actually be ...

env:
  PY_VER:
    matrix:
      - 3.7
      - 3.8
  IRIS_SOURCE:
    matrix:
      - "conda-forge"
      - "source"

Let's see if that works ...

[pp-mo]

No it doesn't like that.

" Failed to parse tasks: 101:11: matrix with a list can only contain maps as it's items!"

I'm going to try your example, though I'm not sure it is right.

Just to put my cards on the table before trying it... according to my reading that approach "ought" to mean that you either include a PY_VER or an IRIS_SOURCE key (but never both) ...

[pp-mo]

(for sake of documenting the struggle...)
Ok, I don't think I was right, but it still isn't what was wanted.
It now tries to launch two tasks called

• py${PY_VER} tests
• py3.7 3.8 tests

[pp-mo]

So I think we are back to either a task with two "env" keys, or an env with two "matrix" keys.
Both equally nasty. :-(

So, following your

I definitely prefer 8762cf5,

I'm going back to the "expand the options" approach.

[trexfeathers]

Well done @pp-mo!

[pp-mo]

@trexfeathers thanks for the discussion, and just for sticking with it!
😬 😄