Skip to content
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

Enhance the unit.py MET testing script to allow for expected failures #2937

Closed
8 of 21 tasks
JohnHalleyGotway opened this issue Jul 18, 2024 · 3 comments · Fixed by #2944
Closed
8 of 21 tasks

Enhance the unit.py MET testing script to allow for expected failures #2937

JohnHalleyGotway opened this issue Jul 18, 2024 · 3 comments · Fixed by #2944
Assignees
Labels
component: testing Software testing issue priority: high High Priority requestor: METplus Team METplus Development Team required: FOR OFFICIAL RELEASE Required to be completed in the official release for the assigned milestone type: new feature Make it do something new
Milestone

Comments

@JohnHalleyGotway
Copy link
Collaborator

JohnHalleyGotway commented Jul 18, 2024

Describe the New Feature

This issue is derived from dtcenter/METplus-Internal#23.

This task is to enhance the logic of the unit.py MET testing script to allow for expected failures.

That script reads one or more input XML files specified as command line arguments. Each XML file consists of one or more tests, each defined using the <test> XML tag. For each test, the <exec> tag specifies the command that should be executed, which is typically a call to a MET executable. If the command run for that test returns non-zero status, then the test fails, prints a failure message, and writes the test output to the screen. For example:

TEST: pcp_combine_sum_GRIB1
	- FAIL - 	1.072 sec
/Volumes/d1/projects/MET/MET_development/MET-feature_2924_fcst_climo/internal/test_unit/../../share/met/../../bin/pcp_combine \
      20120409_00 3 20120409_18 18 \
      /Volumes/d1/projects/MET/MET_unit_test/MET_test_output/pcp_combine/nam_2012040900_F018_APCP18.nc \
      -pcpdir /Volumes/d1/projects/MET/MET_unit_test/MET_test_input/model_data/BADPATH/grib1/namDEBUG 1: Start pcp_combine by johnhg(6088) at 2024-07-18 21:54:49Z  cmd: /Volumes/d1/projects/MET/MET_development/MET-feature_2924_fcst_climo/internal/test_unit/../../share/met/../../bin/pcp_combine 20120409_00 3 20120409_18 18 /Volumes/d1/projects/MET/MET_unit_test/MET_test_output/pcp_combine/nam_2012040900_F018_APCP18.nc -pcpdir /Volumes/d1/projects/MET/MET_unit_test/MET_test_input/model_data/BADPATH/grib1/nam 
DEBUG 2: Performing sum command: Init/In_Accum/Valid/Out_Accum Times = 20120409_000000/030000/20120409_180000/180000
DEBUG 2: Searching for 6 files with accumulation times of 030000 to sum to a total accumulation time of 180000.
ERROR  : 
ERROR  : search_pcp_dir() -> can't open search directory: /Volumes/d1/projects/MET/MET_unit_test/MET_test_input/model_data/BADPATH/grib1/nam
ERROR  : 

This issue is to enhance the control logic to allow for expected failures. So that even though the command may return non-zero status, the test can still pass - if that's what was expected.

Recommend adding a new tag to the MET test XML named <retval>. Use it to specify the expected return value from the <exec> command. Note that <retval> should be optional. If not present, the logic should use a default value of 0, which indicates a successful run.

Update the Python control logic to capture the return value from the <exec> command and compare it against the expected return value (either explicitly specified with <retval> or using the default value of 0 otherwise). If they match, the test succeeds. If they do not match, the test fails, and the stdout should be printed, which is the current behavior.

Acceptance Testing

Please add at least one unit test that uses <retval> to specify an expected non-zero return value to demonstrate this new functionality.

Time Estimate

2 days?

Sub-Issues

Consider breaking the new feature down into sub-issues.

Relevant Deadlines

List relevant project deadlines here or state NONE.

Funding Source

Define the source of funding and account keys here or state NONE.

Define the Metadata

Assignee

  • Select engineer(s) or no engineer required
  • Select scientist(s) or no scientist required

Labels

  • Review default alert labels
  • Select component(s)
  • Select priority
  • Select requestor(s)

Milestone and Projects

  • Select Milestone as a MET-X.Y.Z version, Consider for Next Release, or Backlog of Development Ideas
  • For a MET-X.Y.Z version, select the MET-X.Y.Z Development project

Define Related Issue(s)

Consider the impact to the other METplus components.

New Feature Checklist

See the METplus Workflow for details.

  • Complete the issue definition above, including the Time Estimate and Funding source.
  • Fork this repository or create a branch of develop.
    Branch name: feature_<Issue Number>_<Description>
  • Complete the development and test your changes.
  • Add/update log messages for easier debugging.
  • Add/update unit tests.
  • Add/update documentation.
  • Push local changes to GitHub.
  • Submit a pull request to merge into develop.
    Pull request: feature <Issue Number> <Description>
  • Define the pull request metadata, as permissions allow.
    Select: Reviewer(s) and Development issue
    Select: Milestone as the next official version
    Select: MET-X.Y.Z Development project for development toward the next official release
  • Iterate until the reviewer(s) accept and merge your changes.
  • Delete your fork or branch.
  • Close this issue.
@JohnHalleyGotway JohnHalleyGotway added component: testing Software testing issue type: new feature Make it do something new requestor: METplus Team METplus Development Team required: FOR OFFICIAL RELEASE Required to be completed in the official release for the assigned milestone priority: high High Priority labels Jul 18, 2024
@JohnHalleyGotway JohnHalleyGotway added this to the MET 12.0.0 milestone Jul 18, 2024
@JohnHalleyGotway
Copy link
Collaborator Author

JohnHalleyGotway commented Jul 29, 2024

@natalieb-noaa while you're working on this, please make one additional change to unit.py. The -cmd option prints the environment and command to be run without actually running anything. This is very convenient for testing since it gives me a block of commands to copy/paste onto the command line.

However, there's a small problem in the formatting of the environment commands. The right side of the equal sign needs to be enclosed with single quotes.

For example, the Python version prints no single quotes:

python/unit.py xml/unit_series_analysis.xml -cmd
export CNT_STATS="TOTAL", "ME", "ME_NCL", "ME_NCU"
export CTC_STATS="FY_OY", "FN_ON"
export CTS_STATS="CSI", "GSS"
export FCST_CAT_THRESH=>0.0, >5.0
export FCST_FIELD={ name = "APCP"; level = [ "A06" ]; }
export FHO_STATS="F_RATE", "O_RATE"
export MASK_POLY=
export MCTC_STATS="F1_O1", "F2_O2", "F3_O3"
export MCTS_STATS="ACC", "ACC_NCL", "ACC_NCU"
export MODEL=GFS
export OBS_CAT_THRESH=>0.0, >5.0
export OBS_FIELD={ name = "APCP"; level = [ "A06" ]; }
export OBTYPE=STAGE4
export PCT_STATS=
export PJC_STATS=
export PRC_STATS=
export PSTD_STATS=
export SAL1L2_STATS=
export SL1L2_STATS="FBAR", "OBAR"
...

Whereas the PERL version includes the surrounding single quotes:

perl/unit.pl xml/unit_series_analysis.xml -cmd
export CNT_STATS='"TOTAL", "ME", "ME_NCL", "ME_NCU"'
export CTC_STATS='"FY_OY", "FN_ON"'
export CTS_STATS='"CSI", "GSS"'
export FCST_CAT_THRESH='>0.0, >5.0'
export FCST_FIELD='{ name = "APCP"; level = [ "A06" ]; }'
export FHO_STATS='"F_RATE", "O_RATE"'
export MASK_POLY=''
export MCTC_STATS='"F1_O1", "F2_O2", "F3_O3"'
export MCTS_STATS='"ACC", "ACC_NCL", "ACC_NCU"'
export MODEL='GFS'
export OBS_CAT_THRESH='>0.0, >5.0'
export OBS_FIELD='{ name = "APCP"; level = [ "A06" ]; }'
export OBTYPE='STAGE4'
export PCT_STATS=''
export PJC_STATS=''
export PRC_STATS=''
export PSTD_STATS=''
export SAL1L2_STATS=''
export SL1L2_STATS='"FBAR", "OBAR"'
...

Please add the quotes to the -cmd output to make the copy/paste usage work as expected.

@natalieb-noaa
Copy link
Contributor

@JohnHalleyGotway I've finally read through the details of this issue, and both the main feature request and your additional request in the comment are straightforward and I can take care of them within the next week or two.

@natalieb-noaa
Copy link
Contributor

the above request (adding single quotes around env variables in command mode) has been addressed and committed to feature branch

@JohnHalleyGotway JohnHalleyGotway linked a pull request Aug 13, 2024 that will close this issue
17 tasks
@JohnHalleyGotway JohnHalleyGotway moved this from 🔖 Ready to 👀 In review in MET-12.0.0 Development Aug 13, 2024
@github-project-automation github-project-automation bot moved this from 🔎 In review to 🏁 Done in MET-12.0.0 Development Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: testing Software testing issue priority: high High Priority requestor: METplus Team METplus Development Team required: FOR OFFICIAL RELEASE Required to be completed in the official release for the assigned milestone type: new feature Make it do something new
Projects
No open projects
Status: 🏁 Done
Development

Successfully merging a pull request may close this issue.

2 participants