tests: properly register and skip translate test for GFSPhysicsDriver#65
Merged
jjuyeonkim merged 3 commits intoOct 10, 2025
Merged
Conversation
romanc
added a commit
to romanc/NDSL
that referenced
this pull request
Oct 10, 2025
while it is expected for a test to fail if that happens, this isn't proper usage of the `xfail` marker. because doing so will just hide that tests aren't running, e.g. see NOAA-GFDL/pySHiELD#65.
7 tasks
7 tasks
romanc
commented
Oct 10, 2025
Collaborator
Author
romanc
left a comment
There was a problem hiding this comment.
Just some comment on the proposed changes.
| from .translate_fillgfs import TranslateFillGFS | ||
| from .translate_fpvs import TranslateFPVS | ||
| from .translate_fv_update_phys import DycoreState, TranslateFVUpdatePhys | ||
| from .translate_gfs_physics_driver import TranslateGFSPhysicsDriver |
Collaborator
Author
There was a problem hiding this comment.
This magic import properly registers the translate test.
|
|
||
| import ndsl.dsl.gt4py_utils as utils | ||
| import ndsl.initialization as util | ||
| from ndsl import QuantityFactory, SubtileGridSizer |
Collaborator
Author
There was a problem hiding this comment.
This is cleanup needed to get the translate test to run. ndsl.initialization currently doesn't expose QuantityFactory and SubtileGridSizer (to be fixed with NOAA-GFDL/NDSL#259). Importing everything from top-level ndsl is where we want to go and it will make our lives easier with upcoming refactors such as NOAA-GFDL/NDSL#243.
| self.namelist, | ||
| do_dry_convective_adjust=False, | ||
| dycore_only=self.namelist.dycore_only, | ||
| dycore_only=False, |
Collaborator
Author
There was a problem hiding this comment.
There's no dycore_only attribute in PhysicsConfig. I guess False is a good value since we are in pySHiELD ...
fmalatino
approved these changes
Oct 10, 2025
jjuyeonkim
approved these changes
Oct 10, 2025
github-merge-queue Bot
pushed a commit
to NOAA-GFDL/NDSL
that referenced
this pull request
Oct 10, 2025
* raise if translate test can't be found while it is expected for a test to fail if that happens, this isn't proper usage of the `xfail` marker. because doing so will just hide that tests aren't running, e.g. see NOAA-GFDL/pySHiELD#65. * allow skip_test to be passed as ctor argument skip_test is an option of our translate test system. let's be nice an allow users of the translate test system to use that option as part of the constructor arguments. * cleanup exports from ndsl.initialization --------- Co-authored-by: Roman Cattaneo <1116746+romanc@users.noreply.github.com>
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The translate test for
GFSPhysicsDrivercurrently isn't properly registered. NDSL's translate test system thus automatically marks the test asxfail(expected failure) because it can't find the translate test class.xfails are easy to oversee and don't generally raise alarm (because - after all - the test is expected to fail and it does so).xfailis generally used to test that missing features raise a warning. The behavior only started to show after some typing work, which made things more strict.Given the above context, this PR
GFSPhysicDriveras a translate test,GFSPhysicsDrivertest because it's failing in numerical validation (to be unskipped with issue #66)xfailand insteadraisean exception if something's off with translate tests: Translate tests: raise if test class can't be found NDSL#259.How Has This Been Tested?
Running translate tests locally (with the numpy backend) and on the CI (as always).
Checklist:
N/A
N/A
N/A