Skip to content

Conversation

james-bruten-mo
Copy link
Contributor

@james-bruten-mo james-bruten-mo commented Aug 19, 2025

Description

Summary

Refactor validate_rose_meta.py and move it to SimSys_Scripts. The script will now automatically find metadata sections and apps to validate (ignoring a list of exceptions) rather than only testing based on a hardcoded list. This was done previously and meant that new metadata sections were regularly not tested.

Coordinated merge

This ticket needs to be committed before:
LFRic Apps #954
LFRic Core #4666

Checklist

  • I have performed a self-review of my own changes

@james-bruten-mo james-bruten-mo requested a review from a team as a code owner August 19, 2025 13:30
@james-bruten-mo james-bruten-mo requested review from r-sharp and cameronbateman-mo and removed request for a team August 19, 2025 13:30
@james-bruten-mo james-bruten-mo requested review from jennyhickson and removed request for r-sharp August 21, 2025 08:23
"""

print("\n\n[INFO] - Validating rose-stem apps\n\n")
failed = False
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
failed = False

if errors:
print(f"[FAIL] - {app} failed to validate")
print(f"Failure validating app {app}:\n{errors}", file=sys.stderr)
failed = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
failed = True
return False

else:
print(f"[PASS] - {app} validated")

return failed
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return failed
return True

"""

print("\n\n[INFO] - Checking rose metadata sections\n\n")
failed = False
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
failed = False

f"Failure running rose metata-check on {section}:\n{result.stderr}",
file=sys.stderr,
)
failed = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
failed = True
return False

else:
print(f"[PASS] - {section} validated")

return failed
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return failed
return True

Copy link
Contributor

@cameronbateman-mo cameronbateman-mo left a comment

Choose a reason for hiding this comment

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

Made some suggestions for the return values, I found returning a variable named failures with boolean confusing so suggestions should just return True or False.

Other than that all looks good.

@james-bruten-mo
Copy link
Contributor Author

Thanks Cameron, I've committed the bottom suggestion which is a good shout. I've not done the other ones. The reason for storing the state in the variable is it lets us run the test for all apps before raising an error, so we find all the issues immediately rather than fixing one and running again

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.

2 participants