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

More fine-grain control for QA checks on development branches #99

Open
jo-basevi opened this issue Feb 7, 2025 · 6 comments
Open

More fine-grain control for QA checks on development branches #99

jo-basevi opened this issue Feb 7, 2025 · 6 comments
Assignees

Comments

@jo-basevi
Copy link
Collaborator

As brought up by @anton-seaice in ACCESS-NRI/access-om3-configs#162, some of the QA tests are too onerous for configurations during development.

Currently with how the tests are set up, pytest markers are used to specify what tests to run. There is some configuration options in the config/ci.json file in the main branch of model-configs repositories (e.g., https://github.com/ACCESS-NRI/access-om3-configs/blob/main/config/ci.json and some docs on how to modify this file). This file defines the default pytest markers to run for each style of tests (e.g. qa), and options to specify a specific marker for the target branch. E.g. the below config would use "some_marker" for QA pytests for PRs that are being merged into dev-1deg_jra55do_ryf:

{
    # ...
    "qa": {
        "dev-1deg_jra55do_ryf": { 
            "markers": "some_marker"
        },
        "default": {
           "markers": "access_om3"
       }
}

One improvement would be to allow regex matching, e.g. "dev-*", rather than having to specify many branches. This will require some modifications to the action that parses this config files: https://github.com/ACCESS-NRI/model-config-tests/blob/main/.github/actions/parse-ci-config/action.yml

The other thing will be figuring out a list of QA checks that we might not want to run on dev branches - some examples are already listed in the above linked issue are:

  • test_runlog_is_on
  • test_restart_freq_is_date_based
  • test_metadata_contains_field for 'reference' - maybe just have the metadata tests that check there isn't an experiment UUID?

I guess the most minimal change would be adding a @pytest.mark.config_dev (or similar) decorator above tests to run for configs early in development, and adding this marker here:

config.addinivalue_line(
"markers", "config: mark as configuration tests in quick QA CI checks"
)

@anton-seaice
Copy link
Contributor

@pytest.mark.config_dev (or similar) decorator

I think we need this anyway, so that dev- and release- branches can use different markers. Would the marker be used for all dev- prefix branches in all configurations ? or is the meaning that these configurations are in alpha / beta development and the marker would be removed once that repository contains finalised released configurations?

  • test_metadata_contains_field for 'reference' - maybe just have the metadata tests that check there isn't an experiment UUID?

I think we need to remove this, because by definition the configuration needs releasing before a paper is written about it.

@jo-basevi
Copy link
Collaborator Author

Would the marker be used for all dev- prefix branches in all configurations ? or is the meaning that these configurations are in alpha / beta development and the marker would be removed once that repository contains finalised released configurations?

I think for configurations in alpha/beta development as once there's a release- branch associated with a dev- branch, it probably should be passing the full set of QA checks (?). So using the config/ci.json, it would need to specify this dev marker for each configuration that is in alpha/beta development.

@anton-seaice
Copy link
Contributor

My preference would be release- branches pass the full set marked by the config marker (which includes all the config_dev tests) and dev- branches pass a limited set marked by the config_dev marker, and that that is the state for all configuration repositories :-)

@anton-seaice
Copy link
Contributor

Oh i think we just said the same thing :-)

@anton-seaice
Copy link
Contributor

anton-seaice commented Feb 19, 2025

One improvement would be to allow regex matching, e.g. "dev-*", rather than having to specify many branches. This will require some modifications to the action that parses this config files: https://github.com/ACCESS-NRI/model-config-tests/blob/main/.github/actions/parse-ci-config/action.yml

Is this being worked on ? Planned to be worked on in the short(ish) term ?

@jo-basevi
Copy link
Collaborator Author

Not currently worked on - but I'll try have a look at it today to try quickly get something working.

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

No branches or pull requests

2 participants