Add running the unittests to the GitHub workflow by zmaalick · Pull Request #321 · MetOffice/CMEW

Zubair Maalick (zmaalick) · 2026-01-08T13:25:48Z

Closes #165 .

PR creation checklist for the developer

Has <issue_number> above ☝️ been replaced with the issue number?
Has main been selected as the base branch?
Does the feature branch name follow the format <issue_number>_<short_description_of_feature>?
Does the text of the PR title exactly match with the text (not including the issue number) of the issue title?
Have appropriate reviewers been added to the PR (once it is ready for review)?
Has the PR been assigned to the developer(s)?
Have the same labels as on the issue (except for the good first issue label) been added to the PR?
Has the Climate Model Evaluation Workflow (CMEW) project been added to the PR?
Has the appropriate milestone been added to the PR?

Definition of Done for the developer

Does the change in this PR address the above issue / have all acceptance criteria been met?
Does the change in this PR follow the requirements in the wiki: Developer Guide (including copyrights)?
Have new tests related to the change been added?
Do all the GitHub workflow checks pass?
Do all the tests run locally and pass? (Note: the tests are not run by the GitHub workflow, see wiki: Run the tests locally)
Has the API documentation (e.g. docstrings in Python modules) related to the change been updated appropriately?
Has the user documentation (i.e. everything in the doc directory) related to the change been updated appropriately, including the Quick Start section?
Do the HTML pages render correctly? (See wiki: Build the documentation locally)

PR creation checklist for the reviewer

Has <issue_number> above ☝️ been replaced with the issue number?
Has main been selected as the base branch?
Does the feature branch name follow the format <issue_number>_<short_description_of_feature>?
Does the text of the PR title exactly match with the text (not including the issue number) of the issue title?
Have appropriate reviewers been added to the PR (once it is ready for review)?
Has the PR been assigned to the developer(s)?
Have the same labels as on the issue (except for the good first issue label) been added to the PR?
Has the Climate Model Evaluation Workflow (CMEW) project been added to the PR?
Has the appropriate milestone been added to the PR?

Definition of Done for the reviewer

Does the change in this PR address the above issue / have all acceptance criteria been met?
Does the change in this PR follow the requirements in the wiki: Developer Guide (including copyrights)?
Have new tests related to the change been added? N/A
Do all the GitHub workflow checks pass?
Do all the tests run locally and pass? (Note: the tests are not run by the GitHub workflow, see wiki: Run the tests locally)
Has the API documentation (e.g. docstrings in Python modules) related to the change been updated appropriately? N/A
Has the user documentation (i.e. everything in the doc directory) related to the change been updated appropriately including the Quick Start section? N/A
Do the HTML pages render correctly? (See wiki: Build the documentation locally)

mo-nikosbaltas

Although the unit tests block is correct and works, my suggestion is:
Instead of:

eval "$(conda shell.bash hook)"
conda activate cmew
...

Use conda run:
conda run -n cmew cylc validate -O metoffice -O unittest .

This eliminates shell activation issues and is more reliable in Actions.
This also applies to other tests in this file.
As already said this is a suggestion.

     - name: Run Cylc unit tests
        run: |
          cd CMEW
          conda run -n cmew cylc vip -O metoffice -O unittest .

Although not applicable for this issue I would suggest to include some logging by adding at the end:

      - name: Upload cylc-run logs on failure
        if: failure()
        uses: actions/upload-artifact@v4
        with:
          name: cylc-run-logs
          path: ~/cylc-run

Otherwise it is all good for me.

mo-nikosbaltas · 2026-01-09T15:11:58Z

The unit tests should read:

     - name: Run Cylc unit tests
        run: |
          cd CMEW
          conda run -n cmew cylc vip -O metoffice -O unittest .

not
conda run -n cmew cylc validate -O metoffice -O unittest .

In my previous comment I was giving an example for 'cylc validate'. Sorry if I confused you. Also the checks failed when pushing. Can you correct?

Zubair Maalick (zmaalick) · 2026-01-09T16:13:50Z

Although the unit tests block is correct and works, my suggestion is: Instead of:
eval "$(conda shell.bash hook)"
conda activate cmew
...
Use conda run: conda run -n cmew cylc validate -O metoffice -O unittest .

This eliminates shell activation issues and is more reliable in Actions. This also applies to other tests in this file. As already said this is a suggestion.
     - name: Run Cylc unit tests
        run: |
          cd CMEW
          conda run -n cmew cylc vip -O metoffice -O unittest .
Although not applicable for this issue I would suggest to include some logging by adding at the end:
      - name: Upload cylc-run logs on failure
        if: failure()
        uses: actions/upload-artifact@v4
        with:
          name: cylc-run-logs
          path: ~/cylc-run
Otherwise it is all good for me.

Made the changes and also added the logging. Working fine.

mo-nikosbaltas

Modify PR title to : #321: Add running the unittests to the GitHub workflow. See https://github.com/MetOffice/CMEW/wiki/Detailed-Working-Practices#create-a-pull-request

Zubair Maalick (zmaalick) · 2026-01-12T12:23:16Z

Modify PR title to : #321: Add running the unittests to the GitHub workflow. See https://github.com/MetOffice/CMEW/wiki/Detailed-Working-Practices#create-a-pull-request

Done

mo-nikosbaltas · 2026-01-12T12:44:43Z

Zubair Maalick (zmaalick) · 2026-01-12T13:37:14Z

Also, Zubair Maalick (@zmaalick) can you change the copyright years (# (C) Crown Copyright 2022-2025, Met Office.) in the file to 2022-2026.

Done

mo-nikosbaltas

All looks good.

Emma Hogan (ehogan)

Thanks Zubair Maalick (@zmaalick) 🥳

The "end of new line" issue with one of the files in this branch suggests you have not set up the pre-commit hooks. Please set up the pre-commit hooks. Instructions are available at https://github.com/MetOffice/CMEW/wiki/First-time-developer-instructions#first-time-developer-instructions.

Emma Hogan (ehogan)

Thanks Zubair Maalick (@zmaalick) 😊

Now that the pytest configuration has been added, please remove the cd command to be removed from the unittest app.

Emma Hogan (ehogan)

Almost there! 🤪

mo-nikosbaltas

Looks ok.

Emma Hogan (ehogan)

Thanks Zubair Maalick (@zmaalick), just one more outstanding change (and I will add to the working practices that the developer should check the changes in the "Files changed" tab and perform a self-review before requesting a review).

Emma Hogan (ehogan)

Thanks Zubair Maalick (@zmaalick) 🥳

pytest on the command line, cylc vip -O metoffice -O test and cylc vip -O metoffice -O unittest all return the same output 🎉

Emma Hogan (ehogan) · 2026-02-06T19:34:55Z

-       = cmew-esmvaltool-env pytest
+       =cmew-esmvaltool-env pytest
+
+[file:pytest.ini]


I feel it would be clearer if the full path was specified here, rather than specifying file-install-root at the top of this file. Also, would it be possible to use .pytest.ini rather than pytest.ini, please?

Suggested change

[file:pytest.ini]

[file:${CYLC_WORKFLOW_RUN_DIR/.pytest.ini]

Also, would it be possible to use .pytest.ini rather than pytest.ini, please?

Please also update the name of the file in the repository from pytest.ini to .pytest.ini.

As mentioned at #321 (comment) and in a previous retro where we agreed to update the PR checklist with the point "Remember to re-check the Definition of Done after making changes in response to a review.", please re-check the Definition of Done after making this change (particularly the testing).

Hi Emma Hogan (@ehogan)

in `app/unittest' we are doing:

[file:$CYLC_WORKFLOW_RUN_DIR/.pytest.ini] source=git:https://github.com/MetOffice/CMEW.git::pytest.ini::165_add_unittest_in_github_workflow

cylc does not read hidden files. Making pytest.ini to .pytest.ini will not work in cylc. (that is why unitest are failing in cylc)

we need to convert it back to pytest.ini to make it work.

add unitests in github workflow

c6f04fe

Zubair Maalick (zmaalick) self-assigned this Jan 8, 2026

Zubair Maalick (zmaalick) added good first issue Good for newcomers quality assurance Anything related to Quality Assurance (QA) testing Anything related to testing labels Jan 8, 2026

Zubair Maalick (zmaalick) added this to the v0.2.0 (multiple model runs) milestone Jan 8, 2026

Zubair Maalick (zmaalick) requested review from Alistair Sellar (alistairsellar) and mo-nikosbaltas January 8, 2026 13:29

mo-nikosbaltas reviewed Jan 9, 2026

View reviewed changes

Zubair Maalick (zmaalick) added 2 commits January 9, 2026 15:01

update with changess suggested by nik #165

4d2c3be

remove logging block

53a6163

Zubair Maalick (zmaalick) added 5 commits January 9, 2026 15:13

change validate to vip

05b158e

revert back the changes

b3d7579

add logging block

81db099

end of file line

68cd5c9

unittest block changes

7e32edd

mo-nikosbaltas approved these changes Jan 9, 2026

View reviewed changes

mo-nikosbaltas self-requested a review January 12, 2026 12:06

mo-nikosbaltas suggested changes Jan 12, 2026

View reviewed changes

Zubair Maalick (zmaalick) changed the title ~~add unitests in github workflow~~ Add running the unittests to the GitHub workflow Jan 12, 2026

Merge branch 'main' into 165_add_unittest_in_github_workflow

1edde1d

mo-nikosbaltas self-requested a review January 12, 2026 13:30

update copywrite year

d7a4213

mo-nikosbaltas previously approved these changes Jan 12, 2026

View reviewed changes

mo-nikosbaltas self-requested a review January 12, 2026 13:42

Zubair Maalick (zmaalick) added 3 commits January 15, 2026 10:45

declaration correct in pytest conf

de8d97d

add pytest mark

6fda3d8

update with pytest mark

cecaec7

Emma Hogan (ehogan) requested changes Jan 16, 2026

View reviewed changes

Comment thread pytest.ini

Zubair Maalick (zmaalick) added 2 commits January 16, 2026 11:22

remove marking for pytest

feb3d1e

move pytest conf in pyproj and update pytest command in actions

581af08

Emma Hogan (ehogan) requested changes Jan 21, 2026

View reviewed changes

Comment thread CMEW/app/configure_recipe/bin/test_configure_recipe.py

Comment thread CMEW/app/configure_standardise/bin/test_create_request_file.py

Comment thread pyproject.toml Outdated

Comment thread CMEW/site/metoffice.cylc

Comment thread pyproject.toml Outdated

Zubair Maalick (zmaalick) added 2 commits January 21, 2026 11:30

updated the suggested changes

1bca788

pre-commit changes

c6114c3

Emma Hogan (ehogan) requested changes Jan 22, 2026

View reviewed changes

Comment thread CMEW/app/configure_standardise/bin/test_create_request_file.py

Comment thread CMEW/app/configure_recipe/bin/test_configure_recipe.py Outdated

mo-nikosbaltas mentioned this pull request Jan 22, 2026

Enable switching off CDDS extract #300

Closed

34 tasks

Zubair Maalick (zmaalick) added 2 commits January 23, 2026 08:37

revert back the copywrite change

fe6953e

revert back the change

119b6dc

mo-nikosbaltas approved these changes Jan 23, 2026

View reviewed changes

Emma Hogan (ehogan) requested changes Jan 26, 2026

View reviewed changes

Comment thread CMEW/app/configure_standardise/bin/test_create_request_file.py Outdated

revert the copywrite change

3fc8305

Emma Hogan (ehogan) requested changes Jan 29, 2026

View reviewed changes

Comment thread CMEW/app/unittest/rose-app.conf Outdated

Zubair Maalick (zmaalick) added 5 commits February 3, 2026 13:23

add pytest.ini

1859b12

add unittest solution

59e3a6c

hook changes

1b49c65

update copywrites

e422473

fix rose-dump error

2fc7f4d

Zubair Maalick (zmaalick) requested a review from Emma Hogan (ehogan) February 3, 2026 13:40

Emma Hogan (ehogan) requested changes Feb 6, 2026

View reviewed changes

update unittest rose conf

6a17f12

Zubair Maalick (zmaalick) requested a review from Emma Hogan (ehogan) February 9, 2026 11:29

Emma Hogan (ehogan) requested changes Feb 9, 2026

View reviewed changes

Comment thread CMEW/app/unittest/rose-app.conf Outdated

Comment thread CMEW/app/unittest/rose-app.conf Outdated

fix type

ced2bb3

Zubair Maalick (zmaalick) requested a review from Emma Hogan (ehogan) February 9, 2026 16:50

rename pytest.ini

6e0e98e

Conversation

Zubair Maalick (zmaalick) commented Jan 8, 2026 • edited by mo-nikosbaltas Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR creation checklist for the developer

Definition of Done for the developer

PR creation checklist for the reviewer

Definition of Done for the reviewer

Uh oh!

mo-nikosbaltas left a comment

Choose a reason for hiding this comment

Uh oh!

mo-nikosbaltas commented Jan 9, 2026

Uh oh!

Zubair Maalick (zmaalick) commented Jan 9, 2026

Uh oh!

mo-nikosbaltas left a comment

Choose a reason for hiding this comment

Uh oh!

Zubair Maalick (zmaalick) commented Jan 12, 2026

Uh oh!

mo-nikosbaltas commented Jan 12, 2026

Uh oh!

Zubair Maalick (zmaalick) commented Jan 12, 2026

Uh oh!

mo-nikosbaltas left a comment

Choose a reason for hiding this comment

Uh oh!

Emma Hogan (ehogan) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Emma Hogan (ehogan) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Emma Hogan (ehogan) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mo-nikosbaltas left a comment

Choose a reason for hiding this comment

Uh oh!

Emma Hogan (ehogan) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Emma Hogan (ehogan) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Emma Hogan (ehogan) Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Zubair Maalick (zmaalick) Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Emma Hogan (ehogan) Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Zubair Maalick (zmaalick) Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Emma Hogan (ehogan) Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Zubair Maalick (zmaalick) Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Zubair Maalick (zmaalick) commented Jan 8, 2026 •

edited by mo-nikosbaltas

Loading