Feature dtcenter/METplus-Internal#24 test failure (#1895)

.github/actions/run_tests/entrypoint.sh

@@ -61,7 +61,7 @@ if [[ "$INPUT_CATEGORIES" == pytests* ]]; then
   for x in `cat $PYTESTS_GROUPS_FILEPATH`; do
     marker="${x//_or_/ or }"
     marker="${marker//not_/not }"
-    command+="/usr/local/envs/pytest/bin/pytest -vv --cov=../../metplus -m \"$marker\""
+    command+="/usr/local/envs/pytest/bin/pytest -vv --cov=../../../metplus -m \"$marker\""
     command+=";if [ \$? != 0 ]; then status=1; fi;"
   done
   command+="if [ \$status != 0 ]; then echo ERROR: Some pytests failed. Search for FAILED to review; false; fi"

.github/parm/pytest_groups.txt

@@ -1,3 +1,4 @@
+run_metplus
 util
 wrapper
 wrapper_a

internal/tests/pytests/conftest.py

@@ -4,6 +4,7 @@
 import subprocess
 import pytest
 import getpass
+import shutil
 from pathlib import Path
 
 # add METplus directory to path so the wrappers and utilities can be found
@@ -19,7 +20,8 @@
 if pytest_host is None:
     import socket
     pytest_host = socket.gethostname()
-    print(f"No hostname provided with METPLUS_PYTEST_HOST, using {pytest_host}")
+    print("No hostname provided with METPLUS_PYTEST_HOST, "
+          f"using {pytest_host}")
 else:
     print(f"METPLUS_PYTEST_HOST = {pytest_host}")
 
@@ -33,7 +35,8 @@
 
 # source minimum_pytest.<pytest_host>.sh script
 current_user = getpass.getuser()
-command = shlex.split(f"env -i bash -c 'export USER={current_user} && source {minimum_pytest_file} && env'")
+command = shlex.split(f"env -i bash -c 'export USER={current_user} && "
+                      f"source {minimum_pytest_file} && env'")
 proc = subprocess.Popen(command, stdout=subprocess.PIPE)
 
 for line in proc.stdout:
@@ -43,21 +46,70 @@
 
 proc.communicate()
 
+output_base = os.environ['METPLUS_TEST_OUTPUT_BASE']
+if not output_base:
+    print('ERROR: METPLUS_TEST_OUTPUT_BASE must be set to a path to write')
+    sys.exit(1)
+
+test_output_dir = os.path.join(output_base, 'test_output')
+if os.path.exists(test_output_dir):
+    print(f'Removing test output dir: {test_output_dir}')
+    shutil.rmtree(test_output_dir)
+
+
+@pytest.hookimpl(tryfirst=True, hookwrapper=True)
+def pytest_runtest_makereport(item, call):
+    """! This is used to capture the status of a test so the metplus_config
+    fixture can remove output data from tests that pass.
+    """
+    # execute all other hooks to obtain the report object
+    outcome = yield
+    rep = outcome.get_result()
+
+    # set a report attribute for each phase of a call, which can
+    # be "setup", "call", "teardown"
+
+    setattr(item, "rep_" + rep.when, rep)
+
+
+@pytest.fixture()
+def metplus_config(request):
+    """! Create a METplus configuration object using only the minimum required
+    settings found in minimum_pytest.conf. This fixture checks the result of
+    the test it is used in and automatically removes the output that is
+    generated by it unless the test fails. This makes it much easier to review
+    the failed tests. To use this fixture, add metplus_config to the test
+    function arguments and set a variable called config to metplus_config, e.g.
+    config = metplus_config.
+    """
+    script_dir = os.path.dirname(__file__)
+    args = [os.path.join(script_dir, 'minimum_pytest.conf')]
+    config = config_metplus.setup(args)
+    yield config
+
+    # don't remove output base if test fails
+    if request.node.rep_call.failed:
+        return
+    config_output_base = config.getdir('OUTPUT_BASE')
+    if config_output_base and os.path.exists(config_output_base):
+        shutil.rmtree(config_output_base)
+
+
 @pytest.fixture(scope='function')
-def metplus_config():
-    """! Create a METplus configuration object that can be
-    manipulated/modified to
-         reflect different paths, directories, values, etc. for individual
-         tests.
+def metplus_config_files():
+    """! Create a METplus configuration object using minimum_pytest.conf
+    settings and any list of config files.The metplus_config fixture is
+    preferred because it automatically cleans up the output files generated
+    by the use case unless the test fails. To use this in a test, add
+    metplus_config_files as an argument to the test function and pass in a list
+    of config files to it. Example: config = metplus_config_files([my_file])
     """
-    def read_configs(extra_configs=[]):
+    def read_configs(extra_configs):
         # Read in minimum pytest config file and any other extra configs
         script_dir = os.path.dirname(__file__)
         minimum_conf = os.path.join(script_dir, 'minimum_pytest.conf')
-        args = [minimum_conf]
-        if extra_configs:
-            args.extend(extra_configs)
-
+        args = extra_configs.copy()
+        args.append(minimum_conf)
         config = config_metplus.setup(args)
         return config
 

internal/tests/pytests/minimum_pytest.conf

@@ -1,6 +1,6 @@
 [config]
 INPUT_BASE = {ENV[METPLUS_TEST_INPUT_BASE]}
-OUTPUT_BASE = {ENV[METPLUS_TEST_OUTPUT_BASE]}
+OUTPUT_BASE = {ENV[METPLUS_TEST_OUTPUT_BASE]}/test_output/{RUN_ID}
 MET_INSTALL_DIR = {ENV[METPLUS_TEST_MET_INSTALL_DIR]}
 TMP_DIR = {ENV[METPLUS_TEST_TMP_DIR]}
 

internal/tests/pytests/minimum_pytest.dakota.sh

@@ -2,12 +2,3 @@ export METPLUS_TEST_INPUT_BASE=/d3/projects/MET/METplus_Data
 export METPLUS_TEST_OUTPUT_BASE=/d3/personal/${USER}/pytest
 export METPLUS_TEST_MET_INSTALL_DIR=/d3/projects/MET/MET_releases/met-9.1_beta3
 export METPLUS_TEST_TMP_DIR=${METPLUS_TEST_OUTPUT_BASE}/tmp
-
-export METPLUS_TEST_EXE_WGRIB2=/usr/local/bin/wgrib2
-export METPLUS_TEST_EXE_CUT=/usr/bin/cut
-export METPLUS_TEST_EXE_TR=/usr/bin/tr
-export METPLUS_TEST_EXE_RM=/bin/rm
-export METPLUS_TEST_EXE_NCAP2=/usr/local/nco/bin/ncap2
-export METPLUS_TEST_EXE_CONVERT=/usr/bin/convert
-export METPLUS_TEST_EXE_NCDUMP=/usr/local/bin/ncdump
-export METPLUS_TEST_EXE_EGREP=/bin/egrep

internal/tests/pytests/minimum_pytest.eyewall.sh

@@ -3,12 +3,3 @@ export METPLUS_TEST_OUTPUT_BASE=/d1/${USER}/pytest
 export METPLUS_TEST_MET_INSTALL_DIR=/usr/local/met-9.0
 #export METPLUS_TEST_MET_INSTALL_DIR=/d1/CODE/MET/MET_releases/met-9.0_beta4
 export METPLUS_TEST_TMP_DIR=${METPLUS_TEST_OUTPUT_BASE}/tmp
-
-export METPLUS_TEST_EXE_WGRIB2=/usr/local/bin/wgrib2
-export METPLUS_TEST_EXE_CUT=/usr/bin/cut
-export METPLUS_TEST_EXE_TR=/usr/bin/tr
-export METPLUS_TEST_EXE_RM=/bin/rm
-export METPLUS_TEST_EXE_NCAP2=/usr/local/nco/bin/ncap2
-export METPLUS_TEST_EXE_CONVERT=/usr/bin/convert
-export METPLUS_TEST_EXE_NCDUMP=/usr/local/bin/ncdump
-export METPLUS_TEST_EXE_EGREP=/bin/egrep

internal/tests/pytests/minimum_pytest.hera.sh

@@ -2,12 +2,3 @@ export METPLUS_TEST_INPUT_BASE=/home/${USER}/metplus_pytests
 export METPLUS_TEST_OUTPUT_BASE=/home/${USER}/metplus_pytests/out
 export METPLUS_TEST_MET_INSTALL_DIR=/contrib/met/8.1
 export METPLUS_TEST_TMP_DIR=/tmp
-
-export METPLUS_TEST_EXE_WGRIB2=/apps/wgrib2/2.0.8/intel/18.0.3.222/bin/wgrib2
-export METPLUS_TEST_EXE_CUT=/usr/bin/cut
-export METPLUS_TEST_EXE_TR=/usr/bin/tr
-export METPLUS_TEST_EXE_RM=/usr/bin/rm
-export METPLUS_TEST_EXE_NCAP2=/apps/nco/4.7.0/intel/18.0.3.051/bin/ncap2
-export METPLUS_TEST_EXE_CONVERT=/usr/bin/convert
-export METPLUS_TEST_EXE_NCDUMP=/apps/netcdf/4.7.0/intel/18.0.5.274/bin/ncdump
-export METPLUS_TEST_EXE_EGREP=/usr/bin/grep

internal/tests/pytests/minimum_pytest.kiowa.sh

@@ -1,14 +1,5 @@
 export METPLUS_TEST_INPUT_BASE=/d1/projects/METplus/METplus_Data
 export METPLUS_TEST_OUTPUT_BASE=/d1/personal/${USER}/pytest
-export METPLUS_TEST_MET_INSTALL_DIR=/usr/local/met-9.0
+export METPLUS_TEST_MET_INSTALL_DIR=/usr/local/met
 #export METPLUS_TEST_MET_INSTALL_DIR=/d1/projects/MET/MET_releases/met-9.0_beta4
 export METPLUS_TEST_TMP_DIR=${METPLUS_TEST_OUTPUT_BASE}/tmp
-#export METPLUS_TEST_TMP_DIR=/tmp
-export METPLUS_TEST_EXE_WGRIB2=/usr/local/bin/wgrib2
-export METPLUS_TEST_EXE_CUT=/usr/bin/cut
-export METPLUS_TEST_EXE_TR=/usr/bin/tr
-export METPLUS_TEST_EXE_RM=/bin/rm
-export METPLUS_TEST_EXE_NCAP2=/usr/local/nco/bin/ncap2
-export METPLUS_TEST_EXE_CONVERT=/usr/bin/convert
-export METPLUS_TEST_EXE_NCDUMP=/usr/local/bin/ncdump
-export METPLUS_TEST_EXE_EGREP=/bin/egrep

internal/tests/pytests/minimum_pytest.venus.sh

@@ -2,12 +2,3 @@ export METPLUS_TEST_INPUT_BASE=/gpfs/dell2/emc/verification/noscrub/$USER/METplu
 export METPLUS_TEST_OUTPUT_BASE=/gpfs/dell2/emc/verification/noscrub/$USER/metplus_test
 export METPLUS_TEST_MET_INSTALL_DIR=/gpfs/dell2/emc/verification/noscrub/$USER/met/9.0_beta4
 export METPLUS_TEST_TMP_DIR=${METPLUS_TEST_OUTPUT_BASE}/tmp
-
-export METPLUS_TEST_EXE_WGRIB2=$WGRIB2
-export METPLUS_TEST_EXE_CUT=cut
-export METPLUS_TEST_EXE_TR=tr
-export METPLUS_TEST_EXE_RM=rm
-export METPLUS_TEST_EXE_NCAP2=ncap2
-export METPLUS_TEST_EXE_CONVERT=convert
-export METPLUS_TEST_EXE_NCDUMP=ncdump
-export METPLUS_TEST_EXE_EGREP=egrep

internal/tests/pytests/plotting/tcmpr_plotter/test_tcmpr_plotter.py

@@ -99,7 +99,7 @@ def set_minimum_config_settings(config):
 )
 @pytest.mark.plotting
 def test_read_loop_info(metplus_config, config_overrides, expected_loop_args):
-    config = metplus_config()
+    config = metplus_config
 
     set_minimum_config_settings(config)
 
@@ -181,7 +181,7 @@ def test_read_loop_info(metplus_config, config_overrides, expected_loop_args):
 @pytest.mark.plotting
 def test_tcmpr_plotter_loop(metplus_config, config_overrides,
                             expected_strings):
-    config = metplus_config()
+    config = metplus_config
 
     set_minimum_config_settings(config)
 
@@ -278,7 +278,7 @@ def test_tcmpr_plotter(metplus_config, config_overrides, expected_string):
         expected_string = f' {expected_string}'
 
     for single_file in [True, False]:
-        config = metplus_config()
+        config = metplus_config
 
         set_minimum_config_settings(config)
 

internal/tests/pytests/pytest.ini

@@ -1,5 +1,6 @@
 [pytest]
 markers =
+    run_metplus: custom marker for testing run_metplus.py script
     util: custom marker for testing metplus/util logic
     wrapper_a: custom marker for testing metplus/wrapper logic - A group
     wrapper_b: custom marker for testing metplus/wrapper logic - B group

internal/tests/pytests/run_metplus/test_run_metplus.py

@@ -0,0 +1,53 @@
+#!/usr/bin/env python3
+
+import pytest
+
+from pathlib import Path
+import os
+import shutil
+from subprocess import run
+
+# get METplus directory relative to this file
+# from this script's directory, go up 4 directories
+METPLUS_DIR = str(Path(__file__).parents[4])
+RUN_METPLUS = os.path.join(METPLUS_DIR, 'ush', 'run_metplus.py')
+EXAMPLE_CONF = os.path.join(METPLUS_DIR, 'parm', 'use_cases',
+                            'met_tool_wrapper', 'Example', 'Example.conf')
+MINIMUM_CONF = os.path.join(METPLUS_DIR, 'internal', 'tests', 'pytests',
+                            'minimum_pytest.conf')
+TEST_OUTPUT_DIR = os.path.join(os.environ['METPLUS_TEST_OUTPUT_BASE'],
+                               'test_output')
+NEW_OUTPUT_BASE = os.path.join(TEST_OUTPUT_DIR, 'run_metplus')
+OUTPUT_BASE_OVERRIDE = f"config.OUTPUT_BASE={NEW_OUTPUT_BASE}"
+
+@pytest.mark.run_metplus
+def test_run_metplus_exists():
+    """! Check that run_metplus.py script exists """
+    assert os.path.exists(RUN_METPLUS)
+
+
+@pytest.mark.parametrize(
+    'command, expected_return_code', [
+        ([RUN_METPLUS], 2),
+        ([RUN_METPLUS, EXAMPLE_CONF], 2),
+        ([RUN_METPLUS, EXAMPLE_CONF, MINIMUM_CONF, OUTPUT_BASE_OVERRIDE], 0),
+    ]
+)
+@pytest.mark.run_metplus
+def test_run_metplus_check_return_code(command, expected_return_code):
+    """! Call run_metplus.py without various arguments and check that the
+    expected value is returned by the script. A successful run should return
+    0 and a failed run should return a non-zero return code, typically 2.
+    """
+    process = run(command)
+    assert process.returncode == expected_return_code
+
+    if os.path.exists(NEW_OUTPUT_BASE):
+        shutil.rmtree(NEW_OUTPUT_BASE)
+
+
+@pytest.mark.run_metplus
+def test_output_dir_is_created():
+    """! Check that the test output directory was created after running tests
+    """
+    assert os.path.exists(TEST_OUTPUT_DIR)