Add pytests and CI for simple configuration checks

[jo-basevi]

Add CI checks for checking config.yaml, metadata.yaml files and simple configuration directory checks, as described in #25.

Closes #25

TODO:

• Add Metadata pytests
• Add new CI job that runs the pytests on PRs merging into release-* and dev-*

[CodeGat]

@jo-basevi what markers would be used for these new tests? config for sure, but do we need any others? I think you were talking about a metadata one? And there is a highres marker but I don't know under what conditions that would be invoked.

[jo-basevi]

Ok there's a bit of test marker explosion.. metadata or config for most branches. If metadata checks will always be required to be run, then could remove the metadata, and just have config for all simple ci checks, to keep things simple?

Otherwise for the high resolution configs (just dev-025deg_jra55_ryf at this stage?), add in an or highres to marker expression.

As checking the metadata realms is model specific, there's also markers for access_om2 and access_om2_bgc. @aidanheerdegen Is there a way to distinguish between access-om2 and access-om2-bgc using config.yaml or similar? As more specific tests are added, the access-om2/access-om2-bgc markers might be used more.

[CodeGat]

Sounds good! If we want to run different checks on different branches, we could add another .json file to the config folder on main that informs the CI regarding what markers to use. For example:

{
    "$schema": "./ci.schema.json",
    "pytest": {
        "quick": {
            "markers": {
                "branches": {
                    "dev-025deg_jra55_ryf": "config or metadata or highres"
                },
                "default": "config or metadata"
            }
        },
        "repro": {
            "markers": {
                "branches": {

                },
                "default": "checksum"
            }
        }
    }
}

Also getting @aidanheerdegen s feedback on this would be good

[jo-basevi]

Regrading the checklist in the issue here, the only tests not implemented are:

• all input paths in vk83 (allow exceptions for jra55 data in IAF configs) - Not implemented as unsure on defining what was jra55 data
• all input paths point to files (allow exceptions for IAF forcing data) - Not implemented as some paths were directories and this also required access to gadi's filesystem
• manifests updated and consistent with executables and inputs implemented for model and submodel exe fullpaths but not inputs

[CodeGat]

Alright, got the CI working. Now just need to figure out - which configs need something other than config or metadata, and add them to config/ci.yml (like dev-025deg_jra55_ryf). Tagging @jo-basevi to see if she has some inkling of these things.

[aidanheerdegen]

Is there a way to distinguish between access-om2 and access-om2-bgc using config.yaml or similar? As more specific tests are added, the access-om2/access-om2-bgc markers might be used more.

Well now. I have been mulling over if we should separate the BGC configs out into a separate access-om2-bgc-configs repo. I can see some merit in the idea, even if it is just to reduce branch explosion.

That said, you can tell the difference because the BGC configs will have bgc in their branch name. The CI could check for that and adjust the test suite accordingly I suppose.

[jo-basevi]

Alright, got the CI working. Now just need to figure out - which configs need something other than config or metadata, and add them to config/ci.yml (like dev-025deg_jra55_ryf). Tagging @jo-basevi to see if she has some inkling of these things.

I think the dev-025deg_jra55_ryf is the only high resolution one currently, so:
dev-025deg_jra55_ryf : config or metadata or highres or access_om2
every other access-om2 branch: config or metadata or access_om2

[aidanheerdegen]

Note

#25 (comment)

[jo-basevi]

So call-pr-1-ci.yaml is not triggered on changes to metadata.yaml file here, but metadata files are now being tested on in the QA checks. So, changes only to metadata.yaml shouldn't trigger a repro test, but also if metadata.yaml was updated to pass the QA tests, those QA tests tests should be rerun? Also noticed that changing the metadata.yaml will trigger a call-pr-3-bump-tag workflow, but it looks like that will just emit a warning if the version tag already exists?

[CodeGat]

Yeah, I think we should remove the ignored metadata.yaml now that it is being tested. Just need to verify that we can't get into an infinite loop of commits and checks from github-actions, although that looks to be taken care of with the commit-check job in pr-1-ci.yml.
Yeah so it will run pr-3-bump-tag.yml if there is a push to release-* - which can only happen when a PR is merged.

[jo-basevi]

Given this extra check suggestion here, I wonder if its now worth inferring information from the branch name within the tests - rather than adding markers for each different degree..

[CodeGat]

Just preventing merge until I've tested the CI portion on my own org

[aidanheerdegen]

LGTM. Thanks.

[CodeGat]

Note: Require checks on modification to metadata.yaml to succeed before giving the you must bump... dialogue.

[CodeGat]

Note to self...what the heck did I mean when I wrote the above comment...

In other news, I'm adding a commit that lets devs know that they can modify the version in the metadata.yaml, but should use the appropriate !bump command. I feel that for something like the version in metadata.yaml to be so accessible (modifications to the metadata.yaml will be commonplace) we either need to make it known that it is modifiable but not to, or give it some special name (like _version).

[aidanheerdegen]

CI parts LGTM.

[CodeGat]

Just approving since the CI tests are done. i think I went through all the possible paths...

[CodeGat]

I think I'm all done with the changes here (CI wise) :)

[jo-basevi]

I'm done with the pytest changes (unless there are more review suggestions?)

[aidanheerdegen]

LGTM. Thanks for all the work @jo-basevi and @CodeGat