-
Notifications
You must be signed in to change notification settings - Fork 4
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
Conversation
…or different test types and branches/tags
- 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`
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". |
So more specific: I agree it makes the testing configuration more explicit, easier to understand and in a single location. |
There was a problem hiding this 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.
There was a problem hiding this 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
There was a problem hiding this 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.
@dougiesquire we'll be moving a lot of the infra into the model-configs-agnostic |
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
andpython-version
as inputs.I initially added a bunch of
model-config-tests-version
andpython-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 aconfig/ci.json
file on themain
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:
I've added defaults as most checks will use the same
model-config-tests-version
,python-version
fields. The keys inscheduled
are tags to run scheduled checks on, while keys in thereproducibility
andqa
would be target branches. Ifmodel-config-tests-version
/markers
/python-version
is not specified for a branch/tag, fallback todefault
for the test type, and then the globaldefault
.Closes #109