Skip to content

UKCA test suite port to git#1

Merged
Erica Neininger (ericaneininger) merged 24 commits intoMetOffice:mainfrom
james-bruten-mo:ukca_git_test
Nov 4, 2025
Merged

UKCA test suite port to git#1
Erica Neininger (ericaneininger) merged 24 commits intoMetOffice:mainfrom
james-bruten-mo:ukca_git_test

Conversation

@james-bruten-mo
Copy link
Collaborator

@james-bruten-mo James Bruten (james-bruten-mo) commented Sep 24, 2025

This sets up the UKCA test suite to run using git sources. The primary changes are to the extraction of the code. This uses central scripts stored in SimSys_Scripts. This PR also removes the umdp3_checker which is also now centrally installed in SimSys_Scripts.

It's been tested at meto and on the VM. The meto trac.log is below:

Test Suite Results - ukca - ukca_git/run2

Suite Information

Item Value
Suite Name ukca_git/run2
Suite User james.bruten
Workflow Start 2025-10-31T11:08:32
Groups Run scripts
Dependency Reference Main Like
SimSys_Scripts MetOffice/SimSys_Scripts@main True
ukca james-bruten-mo/ukca@ukca_git_test False

Task Information

✅ succeeded tasks - 2
Task State
extract_source succeeded
umdp3_checker succeeded

@james-bruten-mo
Copy link
Collaborator Author

Sorry if you've started looking, just done a minor further commit

Copy link
Contributor

Choose a reason for hiding this comment

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

This mostly looks good on my end of things and thank you for paying attention to use f-strings instead of .format() . Just a couple of really tiny suggestions from my end regarding raising slightly more informative errors, I think it would be good to raise these errors to help developer debug any issues they would get further on down the line.

def get_git_ref(working_copy_path):
proc = Popen(['git', '-C', working_copy_path, 'rev-parse', 'HEAD'], stdout=PIPE, text=True)
if proc.wait():
raise Exception('Could not determine the HEAD commit.')

Choose a reason for hiding this comment

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

Suggested change
raise Exception('Could not determine the HEAD commit.')
raise RuntimeError('Could not determine the HEAD commit.')

Similarly, here I'd raise a runtime error as the if statement verifies whether the process has finished.

@james-bruten-mo
Copy link
Collaborator Author

Thanks Pierre. For the get_site function I'm not sure this is a value error, so I'm going to leave it as it is.
The get_git_ref one should be changed, but I've just removed that function as I probably shouldn't have left it in. It was an early attempt at interacting with a clone, but not used anymore.

@Pierre-siddall
Copy link
Contributor

Thanks Pierre. For the get_site function I'm not sure this is a value error, so I'm going to leave it as it is. The get_git_ref one should be changed, but I've just removed that function as I probably shouldn't have left it in. It was an early attempt at interacting with a clone, but not used anymore.

Yeah that makes sense I though ValueError would be a stretch but I thought I'd jot it down in the review just in case. With this being said everything else looks good so I'm happy to approve this.

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good to go to me now :) .

@james-bruten-mo
Copy link
Collaborator Author

Thanks Pierre, I missed an import of the deleted function, so just removed that

Copy link
Contributor

Choose a reason for hiding this comment

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

All good. One minor typo-in-a-comment suggestion :)

Co-authored-by: Erica Neininger <107684099+ericaneininger@users.noreply.github.com>
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, happy to approve.

@ericaneininger Erica Neininger (ericaneininger) merged commit 312fdd5 into MetOffice:main Nov 4, 2025
1 check passed
@james-bruten-mo James Bruten (james-bruten-mo) deleted the ukca_git_test branch November 4, 2025 13:21
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.

4 participants