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

Update to use model-config-tests and repro test call #111

Merged
merged 6 commits into from
Jun 10, 2024

Conversation

jo-basevi
Copy link
Collaborator

@jo-basevi jo-basevi commented May 28, 2024

Pytests for QA and reproducibility are going to be packaged up in model-config-tests.

Related PR to repro workflow (ACCESS-NRI/reproducibility#28) - adds model-config-tests-version and python-version as inputs.

I initially added a bunch of model-config-tests-version and python-version in the CI workflow files. This makes updating versions error-some. As it would be good just to change the versions in one place, we could have a config/ci.json file on the main branch that stores all this information.
Then thinking about what this could like, it also makes sense to store any information needed to run model-config-tests, e.g. test markers. Custom test markers can be set in the branch workflow code, but this is difficult to find as can't easily search the repository for it.
Another positive is that it makes the workflows (slightly) more generalised, so it'll be easier to make them reusable.

The config file could like:

{
    "$schema": "./ci.schema.json",
    "scheduled": [
        "release-1deg_jra55_ryf-2.0": {},
        "default": 
            {
                "markers": "checksum"
            }
    ],
    "reproducibility": [
        "default": 
            {
                "markers": "checksum"
            }
    ],
    "qa": [
        "default": 
            {
                "markers": "access-om2 or config"
            }
    ],
    "default": {
        "model-config-tests-version": "0.0.1",
        "python-version": "3.11.0"
    }
}

I've added defaults as most checks will use the same model-config-tests-version, python-version fields. The keys in scheduled are tags to run scheduled checks on, while keys in the reproducibility and qa would be target branches. If model-config-tests-version/markers/python-version is not specified for a branch/tag, fallback to default for the test type, and then the global default.

Closes #109

- Added .github/actions/parse-ci-config to parse configuration file for testing config for a given test type and branch/tag
- Added job to pr-1-ci, schedule-2-start, generate-initial-checksums to use parse config action for getting test markers, test and python versions
- schedule-1-start reads tags from ci/config.json that are listed under `scheduled`
@CodeGat CodeGat self-requested a review June 5, 2024 23:57
@aidanheerdegen
Copy link
Member

This is a really neat approach @jo-basevi

I'd definitely like to get some more eyes on this and get some feedback.

If you have time @dougiesquire it would be good to get a model dev perspective, even if it's "not really my problem, I trust you're doing the right thing".

@aidanheerdegen
Copy link
Member

So more specific: I agree it makes the testing configuration more explicit, easier to understand and in a single location.

Copy link
Contributor

@CodeGat CodeGat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks awesome. Just a couple comments.

README-DEV.md Outdated Show resolved Hide resolved
config/ci.schema.json Show resolved Hide resolved
config/ci.schema.json Show resolved Hide resolved
Copy link
Contributor

@CodeGat CodeGat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good :) can't wait to give it a go

Copy link
Collaborator

@dougiesquire dougiesquire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have time @dougiesquire it would be good to get a model dev perspective, even if it's "not really my problem, I trust you're doing the right thing".

Looks good to me - thanks @jo-basevi! I expect there'll be some code duplication when we (finally) set up the equivalent for the ACCESS-OM3 configs. That may be fine, or it might make sense to move the new action elsewhere. Either way, it'll be easiest to make the call once we have the OM3 case set up.

@CodeGat
Copy link
Contributor

CodeGat commented Jun 7, 2024

@dougiesquire we'll be moving a lot of the infra into the model-configs-agnostic model-configs-test repo once merged: see ACCESS-NRI/model-config-tests#23

@jo-basevi jo-basevi merged commit 63b0d1e into main Jun 10, 2024
2 checks passed
@CodeGat CodeGat deleted the 109-Update-to-use-model-config-tests branch June 14, 2024 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update CI tests to use model-config-tests package
4 participants