diff --git a/.github/actions/run_tests/entrypoint.sh b/.github/actions/run_tests/entrypoint.sh index bd2aa579dc..78ce25e086 100644 --- a/.github/actions/run_tests/entrypoint.sh +++ b/.github/actions/run_tests/entrypoint.sh @@ -8,8 +8,6 @@ WS_PATH=$RUNNER_WORKSPACE/$REPO_NAME # set CI jobs directory variable to easily move it CI_JOBS_DIR=.github/jobs -PYTESTS_GROUPS_FILEPATH=.github/parm/pytest_groups.txt - source ${GITHUB_WORKSPACE}/${CI_JOBS_DIR}/bash_functions.sh # get branch name for push or pull request events @@ -34,7 +32,7 @@ fi # running unit tests (pytests) if [[ "$INPUT_CATEGORIES" == pytests* ]]; then - export METPLUS_ENV_TAG="pytest.v5.1" + export METPLUS_ENV_TAG="test.v5.1" export METPLUS_IMG_TAG=${branch_name} echo METPLUS_ENV_TAG=${METPLUS_ENV_TAG} echo METPLUS_IMG_TAG=${METPLUS_IMG_TAG} @@ -56,15 +54,9 @@ if [[ "$INPUT_CATEGORIES" == pytests* ]]; then . echo Running Pytests - command="export METPLUS_PYTEST_HOST=docker; cd internal/tests/pytests;" - command+="status=0;" - for x in `cat $PYTESTS_GROUPS_FILEPATH`; do - marker="${x//_or_/ or }" - marker="${marker//not_/not }" - command+="/usr/local/conda/envs/${METPLUS_ENV_TAG}/bin/pytest -vv --cov=../../../metplus --cov-append -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" + command="export METPLUS_TEST_OUTPUT_BASE=/data/output;" + command+="/usr/local/conda/envs/${METPLUS_ENV_TAG}/bin/pytest internal/tests/pytests -vv --cov=metplus --cov-append --cov-report=term-missing;" + command+="if [ \$? != 0 ]; then echo ERROR: Some pytests failed. Search for FAILED to review; false; fi" time_command docker run -v $WS_PATH:$GITHUB_WORKSPACE --workdir $GITHUB_WORKSPACE $RUN_TAG bash -c "$command" exit $? fi diff --git a/.github/jobs/run_diff_docker.py b/.github/jobs/run_diff_docker.py index 85a3246a6a..2cfa81bd13 100755 --- a/.github/jobs/run_diff_docker.py +++ b/.github/jobs/run_diff_docker.py @@ -24,6 +24,7 @@ OUTPUT_DIR = '/data/output' DIFF_DIR = '/data/diff' + def copy_diff_output(diff_files): """! Loop through difference output and copy files to directory so it can be made available for comparison. @@ -45,6 +46,7 @@ def copy_diff_output(diff_files): copy_to_diff_dir(diff_file, 'diff') + def copy_to_diff_dir(file_path, data_type): """! Generate output path based on input file path, adding text based on data_type to the filename, then @@ -85,6 +87,7 @@ def copy_to_diff_dir(file_path, data_type): return True + def main(): print('******************************') print("Comparing output to truth data") @@ -97,5 +100,6 @@ def main(): if diff_files: copy_diff_output(diff_files) + if __name__ == '__main__': main() diff --git a/.github/parm/pytest_groups.txt b/.github/parm/pytest_groups.txt deleted file mode 100644 index a5ca80e665..0000000000 --- a/.github/parm/pytest_groups.txt +++ /dev/null @@ -1,8 +0,0 @@ -run_metplus -util -wrapper -wrapper_a -wrapper_b -wrapper_c -wrapper_d -plotting_or_long diff --git a/docs/Contributors_Guide/testing.rst b/docs/Contributors_Guide/testing.rst index f0a4b06f44..1229359804 100644 --- a/docs/Contributors_Guide/testing.rst +++ b/docs/Contributors_Guide/testing.rst @@ -9,20 +9,67 @@ directory. Unit Tests ---------- -Unit tests are run with pytest. They are found in the *pytests* directory. +Unit tests are run with pytest. +They are found in the *internal/tests/pytests* directory under the *wrappers* +and *util* directories. Each tool has its own subdirectory containing its test files. -Unit tests can be run by running the 'pytest' command from the -internal/tests/pytests directory of the repository. -The 'pytest' Python package must be available. +Pytest Requirements +^^^^^^^^^^^^^^^^^^^ + +The following Python packages are required to run the tests. + +* **pytest**: Runs the tests +* **python-dateutil**: Required to run METplus wrappers +* **netCDF4**: Required for some METplus wrapper functionality +* **pytest-cov** (optional): Only if generating code coverage stats +* **pillow** (optional): Only used if running diff utility tests +* **pdf2image** (optional): Only used if running diff utility tests + +Running +^^^^^^^ + +To run the unit tests, set the environment variable +**METPLUS_TEST_OUTPUT_BASE** to a path where the user running has write +permissions, nativate to the METplus directory, then call pytest:: + + export METPLUS_TEST_OUTPUT_BASE=/d1/personal/${USER}/pytest + cd METplus + pytest internal/tests/pytests + A report will be output showing which pytest categories failed. -When running on a new computer, a **minimum_pytest..sh** -file must be created to be able to run the script. This file contains -information about the local environment so that the tests can run. +To view verbose test output, add the **-vv** argument:: + + pytest internal/tests/pytests -vv + +Code Coverage +^^^^^^^^^^^^^ + +If the *pytest-cov* package is installed, the code coverage report can +be generated from the tests by running:: + + pytest internal/tests/pytests --cov=metplus --cov-report=term-missing + +In addition to the pass/fail report, the code coverage information will be +displayed including line numbers that are not covered by any test. -All unit tests must include one of the custom markers listed in the +Subsetting Tests by Directory +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +A subset of the unit tests can be run by adjusting the path. +Be sure to include the *--cov-append* argument so the results of the run +are appended to the full code coverage results. +To run only the GridStat unit tests:: + + pytest internal/tests/pytests/wrappers/grid_stat --cov=metplus --cov-report=term-missing --cov-append + + +Subsetting Tests by Marker +^^^^^^^^^^^^^^^^^^^^^^^^^^ +Unit tests can include one of the custom markers listed in the internal/tests/pytests/pytest.ini file. Some examples include: + * diff * run_metplus * util * wrapper_a @@ -44,47 +91,52 @@ New pytest markers should be added to the pytest.ini file with a brief description. If they are not added to the markers list, then a warning will be output when running the tests. -There are many unit tests for METplus and false failures can occur if all of -the are attempted to run at once. To run only tests with a given marker, run:: - pytest -m + pytest internal/tests/pytests -m To run all tests that do not have a given marker, run:: - pytest -m "not " + pytest internal/tests/pytests -m "not " + +For example, **if you are running on a system that does not have the additional +dependencies required to run the diff utility tests**, you can run all of the +tests except those by running:: + + pytest internal/tests/pytests -m "not diff" + +Multiple marker groups can be run by using the *or* keyword:: + + pytest internal/tests/pytests -m " or " + +Writing Unit Tests +^^^^^^^^^^^^^^^^^^ + +metplus_config fixture +"""""""""""""""""""""" -Multiple marker groups can be run by using the 'or' keyword:: +Many unit tests utilize a pytest fixture named **metplus_config**. +This is defined in the **conftest.py** file in internal/tests/pytests. +This is used to create a METplusConfig object that contains the minimum +configurations needed to run METplus, like **OUTPUT_BASE**. +Using this fixture in a pytest will initialize the METplusConfig object to use +in the tests. - pytest -m " or " +This also creates a unique output directory for each test where +logs and output files are written. This directory is created under +**$METPLUS_TEST_OUTPUT_BASE**/test_output and is named with the run ID. +If the test passes, then the output directory is automatically removed. +If the test fails, the output directory will not be removed so the content +can be reviewed to debug the issue. +To use it, add **metplus_config** as an argument to the test function:: -Use Case Tests --------------- + def test_something(metplus_config) -Use case tests are run via a Python script called **test_use_cases.py**, -found in the *use_cases* directory. -Eventually the running of these tests will be automated using an external -tool, such as GitHub Actions or Travis CI. -The script contains a list of use cases that are found in the repository. -For each computer that will run the use cases, a -**metplus_test_env..sh** file must exist to set local configurations. -All of the use cases can be run by executing the script -**run_test_use_cases.sh**. The use case test script will output the results -into a directory such as */d1//test-use-case-b*, defined in the -environment file. -If */d1//test-use-case-b* already exists, its content will be copied -over to */d1//test-use-case-a*. If data is found in -the */d1//test-use-case-b* directory already exists, its content -will be copied -over to the */d1//test-use-case-a* directory, the script will prompt -the user to remove those files. -Once the tests have finished running, the output found in the two -directories can be compared to see what has changed. Suggested commands -to run to compare the output will be shown on the screen after completion -of the script. +then set a variable called **config** using the fixture name:: -To see which files and directories are only found in one run:: + config = metplus_config - diff -r /d1/mccabe/test-use-case-a /d1/mccabe/test-use-case-b | grep Only +Additional configuration variables can be set by using the set method:: + config.set('config', key, value) diff --git a/internal/scripts/docker_env/Dockerfile b/internal/scripts/docker_env/Dockerfile index bbc7aa6c4d..6e639d1887 100644 --- a/internal/scripts/docker_env/Dockerfile +++ b/internal/scripts/docker_env/Dockerfile @@ -19,3 +19,8 @@ ARG METPLUS_ENV_VERSION ARG ENV_NAME RUN conda list --name ${ENV_NAME}.${METPLUS_ENV_VERSION} > \ /usr/local/conda/envs/${ENV_NAME}.${METPLUS_ENV_VERSION}/environments.yml + +# remove base environment to free up space +ARG METPLUS_ENV_VERSION +ARG BASE_ENV=metplus_base +RUN conda env remove -y --name ${BASE_ENV}.${METPLUS_ENV_VERSION} diff --git a/internal/scripts/docker_env/Dockerfile.cartopy b/internal/scripts/docker_env/Dockerfile.cartopy index c736c2ea73..079259d51b 100644 --- a/internal/scripts/docker_env/Dockerfile.cartopy +++ b/internal/scripts/docker_env/Dockerfile.cartopy @@ -27,3 +27,8 @@ RUN apt update && apt -y upgrade \ && rm -f cartopy_feature_download.py \ && curl https://raw.githubusercontent.com/SciTools/cartopy/master/tools/cartopy_feature_download.py > cartopy_feature_download.py \ && /usr/local/conda/envs/${ENV_NAME}.${METPLUS_ENV_VERSION}/bin/python3 cartopy_feature_download.py cultural physical + +# remove base environment to free up space +ARG METPLUS_ENV_VERSION +ARG BASE_ENV=metplus_base +RUN conda env remove -y --name ${BASE_ENV}.${METPLUS_ENV_VERSION} diff --git a/internal/scripts/docker_env/README.md b/internal/scripts/docker_env/README.md index 80ff1ff7fb..45083f0e57 100644 --- a/internal/scripts/docker_env/README.md +++ b/internal/scripts/docker_env/README.md @@ -426,9 +426,9 @@ export METPLUS_ENV_VERSION=v5.1 -## pytest.v5.1 (from metplus_base.v5.1) +## test.v5.1 (from metplus_base.v5.1) -This environment is used in automation to run the pytests. It requires all of the +This environment is used in automation to run the pytests and diff tests. It requires all of the packages needed to run all of the METplus wrappers, the pytest package and the pytest code coverage package. @@ -436,10 +436,10 @@ code coverage package. ``` export METPLUS_ENV_VERSION=v5.1 -docker build -t dtcenter/metplus-envs:pytest.${METPLUS_ENV_VERSION} \ +docker build -t dtcenter/metplus-envs:test.${METPLUS_ENV_VERSION} \ --build-arg METPLUS_ENV_VERSION \ - --build-arg ENV_NAME=pytest . -docker push dtcenter/metplus-envs:pytest.${METPLUS_ENV_VERSION} + --build-arg ENV_NAME=test . +docker push dtcenter/metplus-envs:test.${METPLUS_ENV_VERSION} ``` diff --git a/internal/scripts/docker_env/scripts/pytest_env.sh b/internal/scripts/docker_env/scripts/test_env.sh similarity index 72% rename from internal/scripts/docker_env/scripts/pytest_env.sh rename to internal/scripts/docker_env/scripts/test_env.sh index 94e83772a0..922d1f552d 100755 --- a/internal/scripts/docker_env/scripts/pytest_env.sh +++ b/internal/scripts/docker_env/scripts/test_env.sh @@ -1,16 +1,20 @@ #! /bin/sh ################################################################################ -# Environment: pytest.v5.1 -# Last Updated: 2023-01-31 (mccabe@ucar.edu) +# Environment: test.v5.1 +# Last Updated: 2023-07-14 (mccabe@ucar.edu) # Notes: Adds pytest and pytest coverage packages to run unit tests # Added pandas because plot_util test needs it # Added netcdf4 because SeriesAnalysis test needs it +# Added pillow and pdf2image for diff tests # Python Packages: # TODO: update version numbers # pytest==? # pytest-cov==? # pandas==? +# netcdf4==? +# pillow==? +# pdf2image==? # # Other Content: None ################################################################################ @@ -19,7 +23,7 @@ METPLUS_VERSION=$1 # Conda environment to create -ENV_NAME=pytest.${METPLUS_VERSION} +ENV_NAME=test.${METPLUS_VERSION} # Conda environment to use as base for new environment BASE_ENV=metplus_base.${METPLUS_VERSION} @@ -29,3 +33,9 @@ conda install -y --name ${ENV_NAME} -c conda-forge pytest conda install -y --name ${ENV_NAME} -c conda-forge pytest-cov conda install -y --name ${ENV_NAME} -c conda-forge pandas conda install -y --name ${ENV_NAME} -c conda-forge netcdf4 +conda install -y --name ${ENV_NAME} -c conda-forge pillow + +apt-get update +apt-get install -y poppler-utils + +conda install -y --name ${ENV_NAME} -c conda-forge pdf2image diff --git a/internal/tests/pytests/conftest.py b/internal/tests/pytests/conftest.py index 8056e4cfe4..968f96b736 100644 --- a/internal/tests/pytests/conftest.py +++ b/internal/tests/pytests/conftest.py @@ -13,48 +13,23 @@ from metplus.util import config_metplus -# get host from either METPLUS_PYTEST_HOST or from actual host name -# Look for minimum_pytest..sh script to source -# error and exit if not found -pytest_host = os.environ.get('METPLUS_PYTEST_HOST') -if pytest_host is None: - import socket - pytest_host = socket.gethostname() - print("No hostname provided with METPLUS_PYTEST_HOST, " - f"using {pytest_host}") -else: - print(f"METPLUS_PYTEST_HOST = {pytest_host}") - -minimum_pytest_file = os.path.join(os.path.dirname(__file__), - f'minimum_pytest.{pytest_host}.sh') -if not os.path.exists(minimum_pytest_file): - print(f"ERROR: minimum_pytest.{pytest_host}.sh file must exist in " - "pytests directory. Set METPLUS_PYTEST_HOST correctly or " - "create file to run pytests on this host.") - sys.exit(4) - -# source minimum_pytest..sh script -current_user = getpass.getuser() -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: - line = line.decode(encoding='utf-8', errors='strict').strip() - key, value = line.split('=') - os.environ[key] = value - -proc.communicate() - -output_base = os.environ['METPLUS_TEST_OUTPUT_BASE'] +output_base = os.environ.get('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) +try: + 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) + + if not os.path.exists(test_output_dir): + print(f'Creating test output dir: {test_output_dir}') + os.makedirs(test_output_dir) +except PermissionError: + print(f'ERROR: Cannot write to $METPLUS_TEST_OUTPUT_BASE: {output_base}') + sys.exit(2) @pytest.hookimpl(tryfirst=True, hookwrapper=True) diff --git a/internal/tests/pytests/minimum_pytest.conf b/internal/tests/pytests/minimum_pytest.conf index ae6183fe5e..703338e43c 100644 --- a/internal/tests/pytests/minimum_pytest.conf +++ b/internal/tests/pytests/minimum_pytest.conf @@ -1,8 +1,9 @@ [config] -INPUT_BASE = {ENV[METPLUS_TEST_INPUT_BASE]} +INPUT_BASE = {ENV[METPLUS_TEST_OUTPUT_BASE]}/input 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]} +MET_INSTALL_DIR = {ENV[METPLUS_TEST_OUTPUT_BASE]} + +DO_NOT_RUN_EXE = True LOG_LEVEL = DEBUG LOG_LEVEL_TERMINAL = WARNING diff --git a/internal/tests/pytests/minimum_pytest.dakota.sh b/internal/tests/pytests/minimum_pytest.dakota.sh deleted file mode 100644 index 0b66555fa9..0000000000 --- a/internal/tests/pytests/minimum_pytest.dakota.sh +++ /dev/null @@ -1,4 +0,0 @@ -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 diff --git a/internal/tests/pytests/minimum_pytest.docker.sh b/internal/tests/pytests/minimum_pytest.docker.sh deleted file mode 100644 index 1ab0c44f61..0000000000 --- a/internal/tests/pytests/minimum_pytest.docker.sh +++ /dev/null @@ -1,5 +0,0 @@ -# These are the paths from within the docker container, docker-space -export METPLUS_TEST_INPUT_BASE=/data/input -export METPLUS_TEST_OUTPUT_BASE=/data/output -export METPLUS_TEST_MET_INSTALL_DIR=/usr/local -export METPLUS_TEST_TMP_DIR=${METPLUS_TEST_OUTPUT_BASE}/tmp diff --git a/internal/tests/pytests/minimum_pytest.eyewall.sh b/internal/tests/pytests/minimum_pytest.eyewall.sh deleted file mode 100644 index 06a69dd650..0000000000 --- a/internal/tests/pytests/minimum_pytest.eyewall.sh +++ /dev/null @@ -1,5 +0,0 @@ -export METPLUS_TEST_INPUT_BASE=/d1/METplus_Data -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 diff --git a/internal/tests/pytests/minimum_pytest.hera.sh b/internal/tests/pytests/minimum_pytest.hera.sh deleted file mode 100644 index bfb541180d..0000000000 --- a/internal/tests/pytests/minimum_pytest.hera.sh +++ /dev/null @@ -1,4 +0,0 @@ -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 diff --git a/internal/tests/pytests/minimum_pytest.kiowa.sh b/internal/tests/pytests/minimum_pytest.kiowa.sh deleted file mode 100644 index 33cb80aa93..0000000000 --- a/internal/tests/pytests/minimum_pytest.kiowa.sh +++ /dev/null @@ -1,5 +0,0 @@ -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 -#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 diff --git a/internal/tests/pytests/minimum_pytest.seneca.sh b/internal/tests/pytests/minimum_pytest.seneca.sh deleted file mode 100644 index 9fac2a711f..0000000000 --- a/internal/tests/pytests/minimum_pytest.seneca.sh +++ /dev/null @@ -1,4 +0,0 @@ -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 -export METPLUS_TEST_TMP_DIR=${METPLUS_TEST_OUTPUT_BASE}/tmp diff --git a/internal/tests/pytests/minimum_pytest.venus.sh b/internal/tests/pytests/minimum_pytest.venus.sh deleted file mode 100644 index 2c4774e348..0000000000 --- a/internal/tests/pytests/minimum_pytest.venus.sh +++ /dev/null @@ -1,4 +0,0 @@ -export METPLUS_TEST_INPUT_BASE=/gpfs/dell2/emc/verification/noscrub/$USER/METplus/METplus-3.0_sample_data -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 diff --git a/internal/tests/pytests/pytest.ini b/internal/tests/pytests/pytest.ini index 1a9aa7a977..2851d20601 100644 --- a/internal/tests/pytests/pytest.ini +++ b/internal/tests/pytests/pytest.ini @@ -9,3 +9,4 @@ markers = wrapper: custom marker for testing metplus/wrapper logic - all others long: custom marker for tests that take a long time to run plotting: custom marker for tests that involve plotting + diff: custom marker for diff util tests that require additional packages diff --git a/internal/tests/pytests/util/diff_util/test_diff_util.py b/internal/tests/pytests/util/diff_util/test_diff_util.py new file mode 100644 index 0000000000..7ac2c837b4 --- /dev/null +++ b/internal/tests/pytests/util/diff_util/test_diff_util.py @@ -0,0 +1,155 @@ +import pytest + +import os +import shutil +import uuid + +from metplus.util.diff_util import dirs_are_equal, ROUNDING_OVERRIDES +from metplus.util import mkdir_p + +test_output_dir = os.path.join(os.environ['METPLUS_TEST_OUTPUT_BASE'], + 'test_output') + +stat_header = 'VERSION MODEL DESC FCST_LEAD FCST_VALID_BEG FCST_VALID_END OBS_LEAD OBS_VALID_BEG OBS_VALID_END FCST_VAR FCST_UNITS FCST_LEV OBS_VAR OBS_UNITS OBS_LEV OBTYPE VX_MASK INTERP_MTHD INTERP_PNTS FCST_THRESH OBS_THRESH COV_THRESH ALPHA LINE_TYPE' +mpr_line_1 = 'V11.1.0 HRRR ALL_1.25 120000 20220701_200000 20220701_200000 000000 20220701_200000 20220701_200000 HPBL m L0 HPBL m L0 ADPSFC DENVER BILIN 4 NA NA NA NA MPR 5 4 DENVER 39.78616 -104.41425 0 0 2160.80324 1498.06763 AMDAR NA NA NA' +mpr_line_2 = 'V11.1.0 HRRR ALL_1.25 120000 20220701_200000 20220701_200000 000000 20220701_200000 20220701_200000 HPBL m L0 HPBL m L0 ADPSFC DENVER BILIN 4 NA NA NA NA MPR 5 4 DENVER 39.78616 -104.41425 0 0 2160.80324 1498.05994 AMDAR NA NA NA' +file_path_1 = '/some/path/of/fake/file/one' +file_path_2 = '/some/path/of/fake/file/two' +file_path_3 = '/some/path/of/fake/file/three' +csv_header = 'Last Name, First Name, Progress' +csv_val_1 = 'Mackenzie, Stu, 0.9999' +csv_val_2 = 'Kenny-Smith, Ambrose, 0.8977' + + +def create_diff_files(files_a, files_b): + unique_id = str(uuid.uuid4())[0:8] + dir_a = os.path.join(test_output_dir, f'diff_{unique_id}', 'a') + dir_b = os.path.join(test_output_dir, f'diff_{unique_id}', 'b') + mkdir_p(dir_a) + mkdir_p(dir_b) + write_test_files(dir_a, files_a) + write_test_files(dir_b, files_b) + return dir_a, dir_b + + +def write_test_files(dirname, files): + for filename, lines in files.items(): + filepath = os.path.join(dirname, filename) + if os.path.sep in filename: + parent_dir = os.path.dirname(filepath) + mkdir_p(parent_dir) + + with open(filepath, 'w') as file_handle: + for line in lines: + file_handle.write(f'{line}\n') + + +@pytest.mark.parametrize( + 'a_files, b_files, rounding_override, expected_is_equal', [ + # txt both empty dir + ({}, {}, None, True), + # txt A empty dir + ({}, {'filename.txt': ['some', 'text']}, None, False), + # txt B empty dir + ({'filename.txt': ['some', 'text']}, {}, None, False), + # txt both empty file + ({'filename.txt': []}, {'filename.txt': []}, None, True), + # txt A empty file + ({'filename.txt': []}, {'filename.txt': ['some', 'text']}, None, False), + # txt B empty file + ({'filename.txt': ['some', 'text']}, {'filename.txt': []}, None, False), + # stat header columns + ({'filename.stat': [stat_header, mpr_line_1]}, + {'filename.stat': [f'{stat_header} NEW_COLUMN', mpr_line_1]}, + None, False), + # stat number of lines + ({'filename.stat': [stat_header, mpr_line_1]}, + {'filename.stat': [stat_header, mpr_line_1, mpr_line_2]}, + None, False), + # stat number of columns + ({'filename.stat': [stat_header, mpr_line_1]}, + {'filename.stat': [stat_header, f'{mpr_line_1} extra_value']}, + None, False), + # stat string + ({'filename.stat': [stat_header, mpr_line_1]}, + {'filename.stat': [stat_header, mpr_line_1.replace('L0', 'Z0')]}, + None, False), + # stat default precision + ({'filename.stat': [stat_header, mpr_line_1]}, + {'filename.stat': [stat_header, mpr_line_1.replace('39.78616', '39.78615')]}, + None, False), + # stat float override precision + ({'filename.stat': [stat_header, mpr_line_1]}, + {'filename.stat': [stat_header, mpr_line_1.replace('39.78616', '39.78615')]}, + 4, True), + # stat out of order + ({'filename.stat': [stat_header, mpr_line_1, mpr_line_2]}, + {'filename.stat': [stat_header, mpr_line_2, mpr_line_1]}, + 4, True), + # stat version differs + ({'filename.stat': [stat_header, mpr_line_1]}, + {'filename.stat': [stat_header, mpr_line_1.replace('V11.1.0', 'V12.0.0')]}, + None, True), + # file_list A without file_list line + ({'file_list.txt': [file_path_1, file_path_2, file_path_3]}, + {'file_list.txt': ['file_list', file_path_1, file_path_2, file_path_3]}, + None, True), + # file_list B without file_list line + ({'file_list.txt': ['file_list', file_path_1, file_path_2, file_path_3]}, + {'file_list.txt': [file_path_1, file_path_2, file_path_3]}, + None, True), + # file_list out of order + ({'file_list.txt': ['file_list', file_path_1, file_path_2, file_path_3]}, + {'file_list.txt': ['file_list', file_path_2, file_path_3, file_path_1]}, + None, True), + # csv equal + ({'file_list.csv': [csv_header, csv_val_1, csv_val_2]}, + {'file_list.csv': [csv_header, csv_val_1, csv_val_2]}, + None, True), + # csv number of columns A + ({'file_list.csv': [csv_header, csv_val_1, csv_val_2]}, + {'file_list.csv': [f'{csv_header}, Position', f'{csv_val_1}, flute', f'{csv_val_2}, harmonica']}, + None, False), + # csv number of columns B + ({'file_list.csv': [f'{csv_header}, Position', f'{csv_val_1}, flute', f'{csv_val_2}, harmonica']}, + {'file_list.csv': [csv_header, csv_val_1, csv_val_2]}, + None, False), + # csv number of lines A + ({'file_list.csv': [csv_header, csv_val_1, csv_val_2]}, + {'file_list.csv': [csv_header, csv_val_1]}, + None, False), + # csv number of lines B + ({'file_list.csv': [csv_header, csv_val_1]}, + {'file_list.csv': [csv_header, csv_val_1, csv_val_2]}, + None, False), + # csv diff default precision + ({'file_list.csv': [csv_header, csv_val_1, csv_val_2]}, + {'file_list.csv': [csv_header, csv_val_1.replace('0.9999', '0.9998'), csv_val_2]}, + None, False), + # csv diff default precision + ({'file_list.csv': [csv_header, csv_val_1, csv_val_2]}, + {'file_list.csv': [csv_header, csv_val_1.replace('0.9999', '0.9998'), csv_val_2]}, + 3, True), + # csv diff first item + ({'file_list.csv': [csv_header, csv_val_1, csv_val_2]}, + {'file_list.csv': [csv_header, csv_val_1.replace('Mackenzie', 'Art'), csv_val_2]}, + None, False), + ] +) +@pytest.mark.diff +def test_diff_dir_text_files(a_files, b_files, rounding_override, expected_is_equal): + if rounding_override: + for filename in a_files: + ROUNDING_OVERRIDES[filename] = rounding_override + + a_dir, b_dir = create_diff_files(a_files, b_files) + assert dirs_are_equal(a_dir, b_dir) == expected_is_equal + + # pass individual files instead of entire directory + for filename in a_files: + if filename in b_files: + a_path = os.path.join(a_dir, filename) + b_path = os.path.join(b_dir, filename) + assert dirs_are_equal(a_path, b_path) == expected_is_equal + + shutil.rmtree(os.path.dirname(a_dir)) diff --git a/internal/tests/pytests/wrappers/grid_stat/test_grid_stat_wrapper.py b/internal/tests/pytests/wrappers/grid_stat/test_grid_stat_wrapper.py index 71d65503a8..80dfe61170 100644 --- a/internal/tests/pytests/wrappers/grid_stat/test_grid_stat_wrapper.py +++ b/internal/tests/pytests/wrappers/grid_stat/test_grid_stat_wrapper.py @@ -765,6 +765,7 @@ def test_grid_stat_single_field(metplus_config, config_overrides, if item not in wrapper.WRAPPER_ENV_VAR_KEYS] env_var_keys = wrapper.WRAPPER_ENV_VAR_KEYS + missing_env + assert len(all_cmds) == len(expected_cmds) for (cmd, env_vars), expected_cmd in zip(all_cmds, expected_cmds): # ensure commands are generated as expected assert cmd == expected_cmd diff --git a/internal/tests/pytests/wrappers/series_analysis/test_series_analysis.py b/internal/tests/pytests/wrappers/series_analysis/test_series_analysis.py index e5815a5b4c..7f0b7e14db 100644 --- a/internal/tests/pytests/wrappers/series_analysis/test_series_analysis.py +++ b/internal/tests/pytests/wrappers/series_analysis/test_series_analysis.py @@ -901,6 +901,7 @@ def test_get_output_dir(metplus_config, template, storm_id, label, expected_resu @pytest.mark.wrapper_a def test_get_netcdf_min_max(metplus_config): + pytest.skip('Rewrite this test to write a NetCDF file and check vals instead of using file in met install dir') expected_min = 0.0 expected_max = 8.0 diff --git a/internal/tests/pytests/wrappers/stat_analysis/test_stat_analysis.py b/internal/tests/pytests/wrappers/stat_analysis/test_stat_analysis.py index de609612ab..9da87d0faa 100644 --- a/internal/tests/pytests/wrappers/stat_analysis/test_stat_analysis.py +++ b/internal/tests/pytests/wrappers/stat_analysis/test_stat_analysis.py @@ -875,26 +875,6 @@ def test_parse_model_info(metplus_config): == expected_out_stat_filename_template) -@pytest.mark.wrapper_d -def test_run_stat_analysis(metplus_config): - # Test running of stat_analysis - st = stat_analysis_wrapper(metplus_config) - # Test 1 - expected_filename = (st.config.getdir('OUTPUT_BASE')+'/stat_analysis' - '/00Z/MODEL_TEST/MODEL_TEST_20190101.stat') - if os.path.exists(expected_filename): - os.remove(expected_filename) - comparison_filename = (METPLUS_BASE+'/internal/tests/data/stat_data/' - +'test_20190101.stat') - st.c_dict['DATE_BEG'] = datetime.datetime.strptime('20190101', '%Y%m%d') - st.c_dict['DATE_END'] = datetime.datetime.strptime('20190101', '%Y%m%d') - st.c_dict['DATE_TYPE'] = 'VALID' - st._run_stat_analysis({}) - assert os.path.exists(expected_filename) - assert (os.path.getsize(expected_filename) == - os.path.getsize(comparison_filename)) - - @pytest.mark.parametrize( 'data_type, config_list, expected_list', [ ('FCST', '\"0,*,*\"', ['"0,*,*"']), diff --git a/metplus/util/config_metplus.py b/metplus/util/config_metplus.py index b6ab999a79..f898039818 100644 --- a/metplus/util/config_metplus.py +++ b/metplus/util/config_metplus.py @@ -463,6 +463,7 @@ def __init__(self, conf=None): self.add_section('user_env_vars') def __del__(self): + """!When object is deleted, close and remove all log handlers""" handlers = self.logger.handlers[:] for handler in handlers: self.logger.removeHandler(handler) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index f7a613bd2d..4278206c6d 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -13,16 +13,17 @@ IMAGE_EXTENSIONS = [ '.jpg', '.jpeg', + '.png', ] NETCDF_EXTENSIONS = [ '.nc', '.cdf', + '.nc4', ] SKIP_EXTENSIONS = [ '.zip', - '.png', '.gif', '.ix', ] @@ -38,6 +39,13 @@ UNSUPPORTED_EXTENSIONS = [ ] +# keywords to search and skip diff tests if found in file path +# PBL use case can be removed after dtcenter/METplus#2246 is completed +SKIP_KEYWORDS = [ + 'PointStat_fcstHRRR_obsAMDAR_PBLH_PyEmbed', +] + + ### # Rounding Constants ### @@ -91,6 +99,12 @@ def get_file_type(filepath): return 'unknown' +def dirs_are_equal(dir_a, dir_b, debug=False, save_diff=False): + if compare_dir(dir_a, dir_b, debug=debug, save_diff=save_diff): + return False + return True + + def compare_dir(dir_a, dir_b, debug=False, save_diff=False): print('::group::Full diff results:') # if input are files and not directories, compare them @@ -192,6 +206,11 @@ def compare_files(filepath_a, filepath_b, debug=False, dir_a=None, dir_b=None, print(f"file_A: {filepath_a}") print(f"file_B: {filepath_b}\n") + for skip in SKIP_KEYWORDS: + if skip in filepath_a or skip in filepath_b: + print(f'WARNING: Skipping diff that contains keyword: {skip}') + return None + set_rounding_precision(filepath_a) # if file does not exist in dir_b, report difference @@ -222,19 +241,8 @@ def compare_files(filepath_a, filepath_b, debug=False, dir_a=None, dir_b=None, return _handle_image_files(filepath_a, filepath_b, save_diff) # if not any of the above types, use diff to compare - print("Comparing text files") - if not filecmp.cmp(filepath_a, filepath_b): - # if files differ, open files and handle expected diffs - if not compare_txt_files(filepath_a, filepath_b, dir_a, dir_b): - print(f"ERROR: File differs: {filepath_b}") - return filepath_a, filepath_b, 'Text diff', '' - - print("No differences in text files") - return True - else: - print("No differences in text files") + return _handle_text_files(filepath_a, filepath_b, dir_a, dir_b) - return True def set_rounding_precision(filepath): global rounding_precision @@ -248,6 +256,7 @@ def set_rounding_precision(filepath): print(f'Using default rounding precision {DEFAULT_ROUNDING_PRECISION}') rounding_precision = DEFAULT_ROUNDING_PRECISION + def _handle_csv_files(filepath_a, filepath_b): print('Comparing CSV') if not compare_csv_files(filepath_a, filepath_b): @@ -295,6 +304,21 @@ def _handle_image_files(filepath_a, filepath_b, save_diff): return filepath_a, filepath_b, 'Image diff', diff_file +def _handle_text_files(filepath_a, filepath_b, dir_a, dir_b): + print("Comparing text files") + if filecmp.cmp(filepath_a, filepath_b, shallow=False): + print("No differences found from filecmp.cmp") + return True + + # if files differ, open files and handle expected diffs + if not compare_txt_files(filepath_a, filepath_b, dir_a, dir_b): + print(f"ERROR: File differs: {filepath_b}") + return filepath_a, filepath_b, 'Text diff', '' + + print("No differences found from compare_txt_files") + return True + + def compare_pdf_as_images(filepath_a, filepath_b, save_diff=False): try: from pdf2image import convert_from_path @@ -343,9 +367,9 @@ def compare_images(image_a, image_b): nx, ny = image_diff.size for x in range(0, int(nx)): for y in range(0, int(ny)): - pixel = image_diff.getpixel((x, y)) - if pixel != 0 and pixel != (0, 0, 0, 0) and pixel != (0, 0, 0): - print(f"Difference pixel: {pixel}") + diff_pixel = image_diff.getpixel((x, y)) + if not _is_zero_pixel(diff_pixel): + print(f"Difference pixel: {diff_pixel}") diff_count += 1 if diff_count: print(f"ERROR: Found {diff_count} differences between images") @@ -353,6 +377,17 @@ def compare_images(image_a, image_b): return None +def _is_zero_pixel(pixel): + """!Check if difference pixel is 0, which means no differences. + + @param pixel pixel value or tuple if multi-layer image + @returns True if all values are 0 or False if any value is non-zero + """ + if isinstance(pixel, tuple): + return all(val == 0 for val in pixel) + return pixel == 0 + + def save_diff_file(image_diff, filepath_b): rel_path, file_extension = os.path.splitext(filepath_b) diff_file = f'{rel_path}_diff.png' @@ -409,8 +444,8 @@ def _compare_csv_columns(lines_a, lines_b): status = True for num, (line_a, line_b) in enumerate(zip(lines_a, lines_b), start=1): for key in keys_a: - val_a = line_a[key] - val_b = line_b[key] + val_a = line_a[key].strip() + val_b = line_b[key].strip() # prevent error if values are diffs are less than # rounding_precision decimal places # METplus issue #1873 addresses the real problem @@ -433,6 +468,8 @@ def _compare_csv_columns(lines_a, lines_b): def _is_equal_rounded(value_a, value_b): if value_a == value_b: return True + if not _is_number(value_a) or not _is_number(value_b): + return False if _truncate_float(value_a) == _truncate_float(value_b): return True if _round_float(value_a) == _round_float(value_b): @@ -440,6 +477,10 @@ def _is_equal_rounded(value_a, value_b): return False +def _is_number(value): + return value.replace('.', '1').replace('-', '1').strip().isdigit() + + def _truncate_float(value): factor = 1 / (10 ** rounding_precision) return float(value) // factor * factor @@ -463,10 +504,9 @@ def compare_txt_files(filepath_a, filepath_b, dir_a=None, dir_b=None): if not len(lines_a): print("Both text files are empty, so they are equal") return True - else: - print(f"Empty file: {filepath_b}\n" - f"Not empty: {filepath_a}") - return False + print(f"Empty file: {filepath_b}\n" + f"Not empty: {filepath_a}") + return False # filepath_b is not empty but filepath_a is empty elif not len(lines_a): print(f"Empty file: {filepath_a}\n" @@ -490,46 +530,38 @@ def compare_txt_files(filepath_a, filepath_b, dir_a=None, dir_b=None): is_stat_file = lines_a[0].startswith('VERSION') # if it is, save the header columns + header_a = None if is_stat_file: - print("Comparing stat file") + print("Comparing stat files") + # pull out header line and skip VERSION to prevent + # diffs from version number changes header_a = lines_a.pop(0).split()[1:] - else: - header_a = None + header_b = lines_b.pop(0).split()[1:] + if len(header_a) != len(header_b): + print('ERROR: Different number of header columns\n' + f' A: {header_a}\n B: {header_b}') + return False if len(lines_a) != len(lines_b): print(f"ERROR: Different number of lines in {filepath_b}") print(f" File_A: {len(lines_a)}\n File_B: {len(lines_b)}") return False - all_good = diff_text_lines(lines_a, - lines_b, - dir_a=dir_a, - dir_b=dir_b, - print_error=False, - is_file_list=is_file_list, - is_stat_file=is_stat_file, - header_a=header_a) + if diff_text_lines(lines_a, lines_b, dir_a=dir_a, dir_b=dir_b, + print_error=False, is_file_list=is_file_list, + is_stat_file=is_stat_file, header_a=header_a): + return True # if differences found in text file, sort and try again - if not all_good: - lines_a.sort() - lines_b.sort() - all_good = diff_text_lines(lines_a, - lines_b, - dir_a=dir_a, - dir_b=dir_b, - print_error=True, - is_file_list=is_file_list, - is_stat_file=is_stat_file, - header_a=header_a) - - return all_good + lines_a.sort() + lines_b.sort() + return diff_text_lines(lines_a, lines_b, dir_a=dir_a, dir_b=dir_b, + print_error=True, is_file_list=is_file_list, + is_stat_file=is_stat_file, header_a=header_a) -def diff_text_lines(lines_a, lines_b, - dir_a=None, dir_b=None, - print_error=False, - is_file_list=False, is_stat_file=False, +def diff_text_lines(lines_a, lines_b, dir_a=None, dir_b=None, + print_error=False, is_file_list=False, is_stat_file=False, header_a=None): all_good = True for line_a, line_b in zip(lines_a, lines_b): @@ -551,33 +583,46 @@ def diff_text_lines(lines_a, lines_b, continue if print_error: - print(f"ERROR: Line differs\n" - f" A: {compare_a}\n B: {compare_b}") + print(f"ERROR: Line differs\n A: {compare_a}\n B: {compare_b}") all_good = False return all_good -def _diff_stat_line(compare_a, compare_b, header_a, print_error=False): +def _diff_stat_line(compare_a, compare_b, header, print_error=False): """Compare values in .stat file. Ignore first column which contains MET version number @param compare_a list of values in line A @param compare_b list of values in line B - @param header_a list of header values in file A excluding MET version + @param header list of header values in file A excluding MET version @param print_error If True, print an error message if any value differs """ cols_a = compare_a.split()[1:] cols_b = compare_b.split()[1:] + + # error message to print if a diff is found + message = f"ERROR: Stat line differs\n A: {compare_a}\n B: {compare_b}\n\n" + + # error if different number of columns are found + if len(cols_a) != len(cols_b): + if print_error: + print(f'{message}Different number of columns') + return False + all_good = True - for col_a, col_b, label in zip(cols_a, cols_b, header_a): - if col_a == col_b: + for index, (col_a, col_b) in enumerate(zip(cols_a, cols_b), 2): + if _is_equal_rounded(col_a, col_b): continue - if print_error: - print(f"ERROR: {label} differs:\n" - f" A: {col_a}\n B: {col_b}") all_good = False + if not print_error: + continue + + label = f'column {index}' if index >= len(header) else header[index] + message += f" Diff in {label}:\n A: {col_a}\n B: {col_b}\n" + if not all_good and print_error: + print(message) return all_good @@ -724,4 +769,6 @@ def _all_values_are_equal(var_a, var_b): dir_a = sys.argv[1] dir_b = sys.argv[2] save_diff = len(sys.argv) > 3 - compare_dir(dir_a, dir_b, debug=True, save_diff=save_diff) + # if any files were flagged, exit non-zero + if compare_dir(dir_a, dir_b, debug=True, save_diff=save_diff): + sys.exit(2) diff --git a/metplus/wrappers/tcmpr_plotter_wrapper.py b/metplus/wrappers/tcmpr_plotter_wrapper.py index e8408586dc..3e7fe3238e 100755 --- a/metplus/wrappers/tcmpr_plotter_wrapper.py +++ b/metplus/wrappers/tcmpr_plotter_wrapper.py @@ -85,7 +85,12 @@ def create_c_dict(self): # check that R script can be found if not os.path.exists(c_dict['TCMPR_SCRIPT']): - self.log_error('plot_tcmpr.R script could not be found') + self.logger.error('plot_tcmpr.R script could not be found') + + # if running script, set isOK to False + # this allows tests to run without needing MET_INSTALL_DIR + if not c_dict.get('DO_NOT_RUN_EXE', False): + self.isOK = False # get input data c_dict['INPUT_DATA'] = (