From f4e6e783023753a2ec7ed66ca00efec6f4cd41a7 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 12 Jul 2023 12:41:47 -0600 Subject: [PATCH 01/41] set shallow=False for file comparision because shallow compare can incorrectly consider 2 files equal when their content is not --- metplus/util/diff_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index f7a613bd2d..e283d35fa7 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -223,7 +223,7 @@ def compare_files(filepath_a, filepath_b, debug=False, dir_a=None, dir_b=None, # if not any of the above types, use diff to compare print("Comparing text files") - if not filecmp.cmp(filepath_a, filepath_b): + if not filecmp.cmp(filepath_a, filepath_b, shallow=False): # 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}") From 670cabf498b321ea3c5aa6c4fa0b729ecd8feaa5 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 12 Jul 2023 12:45:42 -0600 Subject: [PATCH 02/41] pop header line from file b that was incorrectly removed, add check to error if header values differ --- metplus/util/diff_util.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index e283d35fa7..b456487f5a 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -493,6 +493,11 @@ def compare_txt_files(filepath_a, filepath_b, dir_a=None, dir_b=None): if is_stat_file: print("Comparing stat file") header_a = lines_a.pop(0).split()[1:] + 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 else: header_a = None From 8cbf350bf88564f579f8b9205e288a58a298e3b9 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 12 Jul 2023 12:47:05 -0600 Subject: [PATCH 03/41] remove header from zip so additional columns in stat files past the length of the header columns are still compared, ci-run-all-diff --- metplus/util/diff_util.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index b456487f5a..247919ab88 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -575,11 +575,12 @@ def _diff_stat_line(compare_a, compare_b, header_a, print_error=False): cols_a = compare_a.split()[1:] cols_b = compare_b.split()[1:] all_good = True - for col_a, col_b, label in zip(cols_a, cols_b, header_a): +# for col_a, col_b, label in zip(cols_a, cols_b, header_a): + for col_a, col_b in zip(cols_a, cols_b): if col_a == col_b: continue if print_error: - print(f"ERROR: {label} differs:\n" + print(f"ERROR: value differs:\n" f" A: {col_a}\n B: {col_b}") all_good = False From 8b2fc0eb4922d33804b46687178504acae7dec43 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 12 Jul 2023 17:08:31 -0600 Subject: [PATCH 04/41] error if different number of columns in stat files, include column number to easily identify which columns differ, various clean up --- metplus/util/diff_util.py | 72 +++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 37 deletions(-) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index 247919ab88..0ed1e20234 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -490,51 +490,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:] 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 - else: - header_a = None 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): @@ -556,34 +543,45 @@ 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:] all_good = True -# for col_a, col_b, label in zip(cols_a, cols_b, header_a): - for col_a, col_b in zip(cols_a, cols_b): + # 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 + + for index, (col_a, col_b) in enumerate(zip(cols_a, cols_b), 2): if col_a == col_b: continue - if print_error: - print(f"ERROR: value 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 From 998fd47e3b00e03c1a5091357545f8da1266bfbc Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 12 Jul 2023 17:09:53 -0600 Subject: [PATCH 05/41] ci-run-all-diff --- metplus/util/diff_util.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index 0ed1e20234..9cedf58d45 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -560,7 +560,7 @@ def _diff_stat_line(compare_a, compare_b, header, print_error=False): """ cols_a = compare_a.split()[1:] cols_b = compare_b.split()[1:] - all_good = True + # 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" @@ -570,6 +570,7 @@ def _diff_stat_line(compare_a, compare_b, header, print_error=False): print(f'{message}Different number of columns') return False + all_good = True for index, (col_a, col_b) in enumerate(zip(cols_a, cols_b), 2): if col_a == col_b: continue From f1898b3640b632261b6b4482a0419d285c6d6d55 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Thu, 13 Jul 2023 08:45:45 -0600 Subject: [PATCH 06/41] change log messages to discern which test determined no diffs, turn on pbl use case to test expected failure, ci-run-diff --- .github/parm/use_case_groups.json | 2 +- metplus/util/diff_util.py | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/parm/use_case_groups.json b/.github/parm/use_case_groups.json index faf63b90ad..f7557c8868 100644 --- a/.github/parm/use_case_groups.json +++ b/.github/parm/use_case_groups.json @@ -122,7 +122,7 @@ { "category": "pbl", "index_list": "0", - "run": false + "run": true }, { "category": "precipitation", diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index 9cedf58d45..8e529443a0 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -229,10 +229,10 @@ def compare_files(filepath_a, filepath_b, debug=False, dir_a=None, dir_b=None, print(f"ERROR: File differs: {filepath_b}") return filepath_a, filepath_b, 'Text diff', '' - print("No differences in text files") + print("No differences found from compare_txt_files") return True else: - print("No differences in text files") + print("No differences found from filecmp.cmp") return True @@ -463,10 +463,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" From 8207e4b73416706d69b3e5655ffa8706bc49f1e5 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Thu, 13 Jul 2023 15:03:50 -0600 Subject: [PATCH 07/41] add blank lines for pep8 standards --- .github/jobs/run_diff_docker.py | 4 ++++ 1 file changed, 4 insertions(+) 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() From 6de36208c797e0826651272fcfd62bdde9dc6e0d Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Thu, 13 Jul 2023 15:16:14 -0600 Subject: [PATCH 08/41] refactor to move logic into function, return non-zero if diffs occur when running diff script directly --- metplus/util/diff_util.py | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index 8e529443a0..fb43aaadd1 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -222,19 +222,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, shallow=False): - # 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 - else: - print("No differences found from filecmp.cmp") + return _handle_text_files(filepath_a, filepath_b, dir_a, dir_b) - return True def set_rounding_precision(filepath): global rounding_precision @@ -248,6 +237,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 +285,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 @@ -728,4 +733,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) From e6c8f5f27ca4d37c2db908f075f18c818b23bc97 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Thu, 13 Jul 2023 15:31:35 -0600 Subject: [PATCH 09/41] skip diff if file path contains a keyword, skip PBL use case --- metplus/util/diff_util.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index fb43aaadd1..2a67ff1f81 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -38,6 +38,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 ### @@ -192,6 +199,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 From 6890dc314d874703bab6c276b0c47e09f25ed9ed Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Thu, 13 Jul 2023 15:37:15 -0600 Subject: [PATCH 10/41] use rounding to compare stat lines instead of exact equal --- metplus/util/diff_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index 2a67ff1f81..ef68e7af0c 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -588,7 +588,7 @@ def _diff_stat_line(compare_a, compare_b, header, print_error=False): all_good = True for index, (col_a, col_b) in enumerate(zip(cols_a, cols_b), 2): - if col_a == col_b: + if _is_equal_rounded(col_a, col_b): continue all_good = False if not print_error: From a1caa8ae8a4bdc27b0e8c60811c8d4790f7cf3b5 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Thu, 13 Jul 2023 17:27:58 -0600 Subject: [PATCH 11/41] added function to return boolean if directories are equal --- metplus/util/diff_util.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index ef68e7af0c..b6a5a1754d 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -98,6 +98,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 From 15711ff9860faf43c9a8696b07a115ebabe868ec Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Thu, 13 Jul 2023 17:28:29 -0600 Subject: [PATCH 12/41] per #2244, add unit tests for diff utility --- .../pytests/util/diff_util/test_diff_util.py | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 internal/tests/pytests/util/diff_util/test_diff_util.py 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..f40806b784 --- /dev/null +++ b/internal/tests/pytests/util/diff_util/test_diff_util.py @@ -0,0 +1,95 @@ +import pytest + +import os +import shutil +import uuid + +from metplus.util.diff_util import dirs_are_equal +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' + + +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_files(dir_a, files_a) + write_files(dir_b, files_b) + return dir_a, dir_b + + +def write_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.util +def test_diff_dirs_both_empty(): + a_dir, b_dir = create_diff_files({}, {}) + assert dirs_are_equal(a_dir, b_dir) + shutil.rmtree(os.path.dirname(a_dir)) + + +@pytest.mark.util +def test_diff_dirs_one_empty(): + test_files = {'filename.txt': ['some', 'text']} + for empty_one in ('a', 'b'): + a_files = b_files = {} + if empty_one == 'a': + b_files = test_files + else: + a_files = test_files + a_dir, b_dir = create_diff_files(a_files, b_files) + assert not dirs_are_equal(a_dir, b_dir) + shutil.rmtree(os.path.dirname(a_dir)) + + +@pytest.mark.util +def test_diff_files_both_empty(): + test_files = {'filename.txt': []} + a_dir, b_dir = create_diff_files(test_files, test_files) + assert dirs_are_equal(a_dir, b_dir) + shutil.rmtree(os.path.dirname(a_dir)) + + +@pytest.mark.util +def test_diff_files_one_empty(): + filename = 'filename.txt' + test_content = ['some', 'text'] + for empty_one in ('a', 'b'): + a_files = {filename: []} + b_files = {filename: []} + if empty_one == 'a': + b_files[filename].extend(test_content) + else: + a_files[filename].extend(test_content) + a_dir, b_dir = create_diff_files(a_files, b_files) + assert not dirs_are_equal(a_dir, b_dir) + shutil.rmtree(os.path.dirname(a_dir)) + + +@pytest.mark.util +def test_diff_stat_header_columns(): + filename = 'filename.stat' + a_content = [stat_header, mpr_line_1] + b_content = [f'{stat_header} NEW_COLUMN', mpr_line_1] + a_files = {filename: [a_content]} + b_files = {filename: [b_content]} + a_dir, b_dir = create_diff_files(a_files, b_files) + assert not dirs_are_equal(a_dir, b_dir) + shutil.rmtree(os.path.dirname(a_dir)) From 83b24946e360976fff89161b2f165cf8202a37fb Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Thu, 13 Jul 2023 15:18:59 -0600 Subject: [PATCH 13/41] per #2245, use unique run ID to name logger instance to ensure that concurrent runs do not use the same logger instance --- metplus/util/config_metplus.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/metplus/util/config_metplus.py b/metplus/util/config_metplus.py index f90aeed939..f600bcfc8b 100644 --- a/metplus/util/config_metplus.py +++ b/metplus/util/config_metplus.py @@ -240,7 +240,7 @@ def launch(config_list): config.set('config', 'CONFIG_INPUT', ','.join(config_format_list)) # save unique identifier for the METplus run - config.set('config', 'RUN_ID', str(uuid.uuid4())[0:8]) + config.set('config', 'RUN_ID', config.run_id) # get OUTPUT_BASE to make sure it is set correctly so the first error # that is logged relates to OUTPUT_BASE, not LOG_DIR, which is likely @@ -450,7 +450,8 @@ def __init__(self, conf=None): interpolation=None) if (conf is None) else conf super().__init__(conf) self._cycle = None - self._logger = logging.getLogger('metplus') + self.run_id = str(uuid.uuid4())[0:8] + self._logger = logging.getLogger(f'metplus.{self.run_id}') # config.logger is called in wrappers, so set this name # so the code doesn't break self.logger = self._logger From da6e5b170d0ce2d84a9219e992d4d75f60463799 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Fri, 14 Jul 2023 10:02:54 -0600 Subject: [PATCH 14/41] per #2249, add a step to Dockerfiles to remove the conda environment that was used to create a new environment to free up space --- internal/scripts/docker_env/Dockerfile | 5 +++++ internal/scripts/docker_env/Dockerfile.cartopy | 5 +++++ 2 files changed, 10 insertions(+) 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} From 577147dec0dd343db529718d69903a441ef6dbed Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Fri, 14 Jul 2023 10:18:55 -0600 Subject: [PATCH 15/41] create test.v5.1 metplus-env docker image to use for both pytests and diff tests since the pytests will require the packages needed for the diff tests to properly unit test diff_util --- internal/scripts/docker_env/README.md | 10 +++++----- .../scripts/{pytest_env.sh => test_env.sh} | 16 +++++++++++++--- 2 files changed, 18 insertions(+), 8 deletions(-) rename internal/scripts/docker_env/scripts/{pytest_env.sh => test_env.sh} (72%) 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 From 723a574344393471164a9f6268bb2a1caf691c54 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Fri, 14 Jul 2023 10:22:27 -0600 Subject: [PATCH 16/41] use new test.v5.1 docker image for pytests --- .github/actions/run_tests/entrypoint.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/run_tests/entrypoint.sh b/.github/actions/run_tests/entrypoint.sh index bd2aa579dc..1814e2a995 100644 --- a/.github/actions/run_tests/entrypoint.sh +++ b/.github/actions/run_tests/entrypoint.sh @@ -34,7 +34,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} From 5a9548ad9ff594a6b02178fcafffdc4bcf8e8583 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Fri, 14 Jul 2023 12:06:12 -0600 Subject: [PATCH 17/41] updated comparison that includes rounding to properly handle strings by checking if the value is numeric --- metplus/util/diff_util.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index b6a5a1754d..8dcab9633b 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -456,6 +456,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): @@ -463,6 +465,10 @@ def _is_equal_rounded(value_a, value_b): return False +def _is_number(value): + return value.replace('.','1').replace('-', '1').isdigit() + + def _truncate_float(value): factor = 1 / (10 ** rounding_precision) return float(value) // factor * factor From 159493b1b33b8c09ecb5afaf48e8b039bd7c8986 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Fri, 14 Jul 2023 12:06:36 -0600 Subject: [PATCH 18/41] added unit tests for comparing stat files --- .../pytests/util/diff_util/test_diff_util.py | 80 ++++++++++++++++++- 1 file changed, 77 insertions(+), 3 deletions(-) diff --git a/internal/tests/pytests/util/diff_util/test_diff_util.py b/internal/tests/pytests/util/diff_util/test_diff_util.py index f40806b784..1ed304603d 100644 --- a/internal/tests/pytests/util/diff_util/test_diff_util.py +++ b/internal/tests/pytests/util/diff_util/test_diff_util.py @@ -4,7 +4,7 @@ import shutil import uuid -from metplus.util.diff_util import dirs_are_equal +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'], @@ -88,8 +88,82 @@ def test_diff_stat_header_columns(): filename = 'filename.stat' a_content = [stat_header, mpr_line_1] b_content = [f'{stat_header} NEW_COLUMN', mpr_line_1] - a_files = {filename: [a_content]} - b_files = {filename: [b_content]} + a_files = {filename: a_content} + b_files = {filename: b_content} a_dir, b_dir = create_diff_files(a_files, b_files) assert not dirs_are_equal(a_dir, b_dir) shutil.rmtree(os.path.dirname(a_dir)) + + +@pytest.mark.util +def test_diff_stat_number_of_lines(): + filename = 'filename.stat' + a_content = [stat_header, mpr_line_1] + b_content = [stat_header, mpr_line_1, mpr_line_2] + a_files = {filename: a_content} + b_files = {filename: b_content} + a_dir, b_dir = create_diff_files(a_files, b_files) + assert not dirs_are_equal(a_dir, b_dir) + shutil.rmtree(os.path.dirname(a_dir)) + + +@pytest.mark.util +def test_diff_stat_number_of_columns(): + filename = 'filename.stat' + a_content = [stat_header, mpr_line_1] + b_content = [stat_header, f'{mpr_line_1} extra_value'] + a_files = {filename: a_content} + b_files = {filename: b_content} + a_dir, b_dir = create_diff_files(a_files, b_files) + assert not dirs_are_equal(a_dir, b_dir) + shutil.rmtree(os.path.dirname(a_dir)) + + +@pytest.mark.util +def test_diff_stat_string(): + filename = 'filename.stat' + a_content = [stat_header, mpr_line_1] + b_content = [stat_header, mpr_line_1.replace('L0', 'Z0')] + a_files = {filename: a_content} + b_files = {filename: b_content} + a_dir, b_dir = create_diff_files(a_files, b_files) + assert not dirs_are_equal(a_dir, b_dir) + shutil.rmtree(os.path.dirname(a_dir)) + + +@pytest.mark.util +def test_diff_stat_float_default_precision(): + filename = 'filename.stat' + a_content = [stat_header, mpr_line_1] + b_content = [stat_header, mpr_line_1.replace('39.78616', '39.78615')] + a_files = {filename: a_content} + b_files = {filename: b_content} + a_dir, b_dir = create_diff_files(a_files, b_files) + assert not dirs_are_equal(a_dir, b_dir) + shutil.rmtree(os.path.dirname(a_dir)) + + +@pytest.mark.util +def test_diff_stat_float_override_precision(): + filename = 'filename.stat' + ROUNDING_OVERRIDES[filename] = 4 + a_content = [stat_header, mpr_line_1] + b_content = [stat_header, mpr_line_1.replace('39.78616', '39.78615')] + a_files = {filename: a_content} + b_files = {filename: b_content} + a_dir, b_dir = create_diff_files(a_files, b_files) + assert dirs_are_equal(a_dir, b_dir) + shutil.rmtree(os.path.dirname(a_dir)) + + +@pytest.mark.util +def test_diff_stat_out_of_order(): + filename = 'filename.stat' + ROUNDING_OVERRIDES[filename] = 4 + a_content = [stat_header, mpr_line_1, mpr_line_2] + b_content = [stat_header, mpr_line_2, mpr_line_1] + a_files = {filename: a_content} + b_files = {filename: b_content} + a_dir, b_dir = create_diff_files(a_files, b_files) + assert dirs_are_equal(a_dir, b_dir) + shutil.rmtree(os.path.dirname(a_dir)) From d5ea70c7f9e1dd1c4a7f0b732c9097badfb9e352 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Mon, 17 Jul 2023 14:24:13 -0600 Subject: [PATCH 19/41] per #2245, add METplusConfig class function that is called when object is deleted to close log handlers. This prevents OSError: [Errno 24] Too many open files from running all pytests --- metplus/util/config_metplus.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/metplus/util/config_metplus.py b/metplus/util/config_metplus.py index f600bcfc8b..b6ab999a79 100644 --- a/metplus/util/config_metplus.py +++ b/metplus/util/config_metplus.py @@ -462,6 +462,12 @@ def __init__(self, conf=None): # add section to hold environment variables defined by the user self.add_section('user_env_vars') + def __del__(self): + handlers = self.logger.handlers[:] + for handler in handlers: + self.logger.removeHandler(handler) + handler.close() + def log(self, sublog=None): """! Overrides method in ProdConfig If the sublog argument is From 47795f4e4607980eba279043e39b7ad61d3948a4 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Mon, 17 Jul 2023 14:55:39 -0600 Subject: [PATCH 20/41] add doc info --- metplus/util/config_metplus.py | 1 + 1 file changed, 1 insertion(+) 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) From a9622dfe9f330e7b2d85753f51de466432133d11 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Mon, 17 Jul 2023 14:56:01 -0600 Subject: [PATCH 21/41] call all pytests in single call instead of splitting them up by marker --- .github/actions/run_tests/entrypoint.sh | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/.github/actions/run_tests/entrypoint.sh b/.github/actions/run_tests/entrypoint.sh index 1814e2a995..234240026f 100644 --- a/.github/actions/run_tests/entrypoint.sh +++ b/.github/actions/run_tests/entrypoint.sh @@ -56,15 +56,17 @@ 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_PYTEST_HOST=docker;" + 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" + #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 internal/tests/pytests -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" time_command docker run -v $WS_PATH:$GITHUB_WORKSPACE --workdir $GITHUB_WORKSPACE $RUN_TAG bash -c "$command" exit $? fi From 0e8e7d4a8a4e0a7180b687da56e2faefffee7e2f Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Mon, 17 Jul 2023 15:07:11 -0600 Subject: [PATCH 22/41] purposely break 1 test to ensure automated test captures failure --- .../tests/pytests/wrappers/grid_stat/test_grid_stat_wrapper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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..684892e373 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 @@ -707,7 +707,7 @@ def test_handle_climo_file_variables(metplus_config, config_overrides, {'METPLUS_DISTANCE_MAP_DICT': ('distance_map = {baddeley_p = 1;' 'baddeley_max_dist = 2.3;' 'fom_alpha = 4.5;zhu_weight = 0.5;' - 'beta_value(n) = n * n / 3.0;}')}), + 'beta_value(n) = n * n / 3.0;}x')}), ({'GRID_STAT_FOURIER_WAVE_1D_BEG': '0,4,10', }, {'METPLUS_FOURIER_DICT': 'fourier = {wave_1d_beg = [0, 4, 10];}'}), From c0ad3e419e92054de0dfa44e72d7756a2dd207f7 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Mon, 17 Jul 2023 17:14:51 -0600 Subject: [PATCH 23/41] remove commented code after confirming tests run from single command --- .github/actions/run_tests/entrypoint.sh | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.github/actions/run_tests/entrypoint.sh b/.github/actions/run_tests/entrypoint.sh index 234240026f..47e713bda4 100644 --- a/.github/actions/run_tests/entrypoint.sh +++ b/.github/actions/run_tests/entrypoint.sh @@ -59,14 +59,6 @@ if [[ "$INPUT_CATEGORIES" == pytests* ]]; then command="export METPLUS_PYTEST_HOST=docker;" 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" - #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 internal/tests/pytests -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" time_command docker run -v $WS_PATH:$GITHUB_WORKSPACE --workdir $GITHUB_WORKSPACE $RUN_TAG bash -c "$command" exit $? fi From 8b599def98827b65ffd5079990704b300f1769a6 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Mon, 17 Jul 2023 17:15:15 -0600 Subject: [PATCH 24/41] fix test that was broken on purpose and add check that number of commands are equal --- .../tests/pytests/wrappers/grid_stat/test_grid_stat_wrapper.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 684892e373..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 @@ -707,7 +707,7 @@ def test_handle_climo_file_variables(metplus_config, config_overrides, {'METPLUS_DISTANCE_MAP_DICT': ('distance_map = {baddeley_p = 1;' 'baddeley_max_dist = 2.3;' 'fom_alpha = 4.5;zhu_weight = 0.5;' - 'beta_value(n) = n * n / 3.0;}x')}), + 'beta_value(n) = n * n / 3.0;}')}), ({'GRID_STAT_FOURIER_WAVE_1D_BEG': '0,4,10', }, {'METPLUS_FOURIER_DICT': 'fourier = {wave_1d_beg = [0, 4, 10];}'}), @@ -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 From f30277b01335a2a5a7c812ac9c734ac85d501fa9 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Mon, 17 Jul 2023 17:17:14 -0600 Subject: [PATCH 25/41] turn off all use cases for push events --- .github/parm/use_case_groups.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/parm/use_case_groups.json b/.github/parm/use_case_groups.json index f7557c8868..faf63b90ad 100644 --- a/.github/parm/use_case_groups.json +++ b/.github/parm/use_case_groups.json @@ -122,7 +122,7 @@ { "category": "pbl", "index_list": "0", - "run": true + "run": false }, { "category": "precipitation", From 93d26e67b777b8cefd3666ae687f44e610d69b27 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Mon, 17 Jul 2023 17:51:08 -0600 Subject: [PATCH 26/41] skip test for now to remove dependency on file from MET install dir --- .../pytests/wrappers/series_analysis/test_series_analysis.py | 1 + 1 file changed, 1 insertion(+) 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 From 457987c09222921a9633af77d125231067cf036f Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Mon, 17 Jul 2023 17:51:44 -0600 Subject: [PATCH 27/41] remove test that is not needed -- tests do not need to run MET tools --- .../stat_analysis/test_stat_analysis.py | 20 ------------------- 1 file changed, 20 deletions(-) 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,*,*"']), From 8bb277ecf8eee0c0fbfc894168795abb02f7048f Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Mon, 17 Jul 2023 17:52:57 -0600 Subject: [PATCH 28/41] update logic to allow wrapper to initialize properly if R scripts do not need to be present -- for unit tests and running METplus without executing commands --- metplus/wrappers/tcmpr_plotter_wrapper.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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'] = ( From e1dd5c6e3fb2f6bb5459f4399960a73c2a606489 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Mon, 17 Jul 2023 18:20:15 -0600 Subject: [PATCH 29/41] Removed logic to require minimum_pytest..sh script to run pytests. Instead require METPLUS_TEST_OUTPUT_BASE to be set in the user's environment to run tests. Add logic to ensure that the output base is writable by the user running the pytests or alert user before tests start to run --- .github/actions/run_tests/entrypoint.sh | 2 +- internal/tests/pytests/conftest.py | 43 ++++--------------- internal/tests/pytests/minimum_pytest.conf | 5 +-- .../tests/pytests/minimum_pytest.dakota.sh | 4 -- .../tests/pytests/minimum_pytest.docker.sh | 5 --- .../tests/pytests/minimum_pytest.eyewall.sh | 5 --- internal/tests/pytests/minimum_pytest.hera.sh | 4 -- .../tests/pytests/minimum_pytest.kiowa.sh | 5 --- .../tests/pytests/minimum_pytest.seneca.sh | 4 -- .../tests/pytests/minimum_pytest.venus.sh | 4 -- 10 files changed, 12 insertions(+), 69 deletions(-) delete mode 100644 internal/tests/pytests/minimum_pytest.dakota.sh delete mode 100644 internal/tests/pytests/minimum_pytest.docker.sh delete mode 100644 internal/tests/pytests/minimum_pytest.eyewall.sh delete mode 100644 internal/tests/pytests/minimum_pytest.hera.sh delete mode 100644 internal/tests/pytests/minimum_pytest.kiowa.sh delete mode 100644 internal/tests/pytests/minimum_pytest.seneca.sh delete mode 100644 internal/tests/pytests/minimum_pytest.venus.sh diff --git a/.github/actions/run_tests/entrypoint.sh b/.github/actions/run_tests/entrypoint.sh index 47e713bda4..11de254d32 100644 --- a/.github/actions/run_tests/entrypoint.sh +++ b/.github/actions/run_tests/entrypoint.sh @@ -56,7 +56,7 @@ if [[ "$INPUT_CATEGORIES" == pytests* ]]; then . echo Running Pytests - command="export METPLUS_PYTEST_HOST=docker;" + 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" diff --git a/internal/tests/pytests/conftest.py b/internal/tests/pytests/conftest.py index 8056e4cfe4..067ab4fe05 100644 --- a/internal/tests/pytests/conftest.py +++ b/internal/tests/pytests/conftest.py @@ -13,40 +13,7 @@ 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) @@ -56,6 +23,14 @@ 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}') + try: + os.makedirs(test_output_dir) + except PermissionError: + print(f'ERROR: You cannot write to METPLUS_TEST_OUTPUT_BASE') + sys.exit(2) + @pytest.hookimpl(tryfirst=True, hookwrapper=True) def pytest_runtest_makereport(item, call): diff --git a/internal/tests/pytests/minimum_pytest.conf b/internal/tests/pytests/minimum_pytest.conf index ae6183fe5e..4c4cf264f3 100644 --- a/internal/tests/pytests/minimum_pytest.conf +++ b/internal/tests/pytests/minimum_pytest.conf @@ -1,8 +1,7 @@ [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]} 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 From 702352a2fc9afab022f797dcedb55cd4077872e1 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 18 Jul 2023 07:14:25 -0600 Subject: [PATCH 30/41] error and exit if output base is set to a directory where the user does not have write permissions --- internal/tests/pytests/conftest.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/tests/pytests/conftest.py b/internal/tests/pytests/conftest.py index 067ab4fe05..968f96b736 100644 --- a/internal/tests/pytests/conftest.py +++ b/internal/tests/pytests/conftest.py @@ -18,18 +18,18 @@ 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}') - try: + 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: You cannot write to METPLUS_TEST_OUTPUT_BASE') - sys.exit(2) +except PermissionError: + print(f'ERROR: Cannot write to $METPLUS_TEST_OUTPUT_BASE: {output_base}') + sys.exit(2) @pytest.hookimpl(tryfirst=True, hookwrapper=True) From 01bfe2194a54e688eb16531e3ec1b5354d6ffd6f Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 18 Jul 2023 07:14:45 -0600 Subject: [PATCH 31/41] include another valid NetCDF extension --- metplus/util/diff_util.py | 1 + 1 file changed, 1 insertion(+) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index 8dcab9633b..90e7ef8034 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -18,6 +18,7 @@ NETCDF_EXTENSIONS = [ '.nc', '.cdf', + '.nc4', ] SKIP_EXTENSIONS = [ From 3e6616a16c272dba74bca9f330de633108a136d9 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 18 Jul 2023 07:15:09 -0600 Subject: [PATCH 32/41] fix logic to check if a pixel in an image has no differences --- metplus/util/diff_util.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index 90e7ef8034..f1ec2820bd 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -367,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") @@ -377,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' From 596a8ef5d12e19c3b0dd0634a0245b18531d1161 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 18 Jul 2023 07:15:43 -0600 Subject: [PATCH 33/41] test if png diff logic will work, ci-run-all-diff --- metplus/util/diff_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index f1ec2820bd..5d028a9d88 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -13,6 +13,7 @@ IMAGE_EXTENSIONS = [ '.jpg', '.jpeg', + '.png', ] NETCDF_EXTENSIONS = [ @@ -23,7 +24,6 @@ SKIP_EXTENSIONS = [ '.zip', - '.png', '.gif', '.ix', ] From 26562d552cedeaca8767f5bcc8c71931080394aa Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 18 Jul 2023 09:40:02 -0600 Subject: [PATCH 34/41] strip off whitespace before diffing column values --- metplus/util/diff_util.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/metplus/util/diff_util.py b/metplus/util/diff_util.py index 5d028a9d88..4278206c6d 100755 --- a/metplus/util/diff_util.py +++ b/metplus/util/diff_util.py @@ -444,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 @@ -478,7 +478,7 @@ def _is_equal_rounded(value_a, value_b): def _is_number(value): - return value.replace('.','1').replace('-', '1').isdigit() + return value.replace('.', '1').replace('-', '1').strip().isdigit() def _truncate_float(value): From 3b906a3d264c404af9fca66baa614a04e7531016 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 18 Jul 2023 09:40:35 -0600 Subject: [PATCH 35/41] add more tests for csv and stat files. move multiple tests into a single parameterized test --- .../pytests/util/diff_util/test_diff_util.py | 239 ++++++++---------- 1 file changed, 112 insertions(+), 127 deletions(-) diff --git a/internal/tests/pytests/util/diff_util/test_diff_util.py b/internal/tests/pytests/util/diff_util/test_diff_util.py index 1ed304603d..7e1ddca20f 100644 --- a/internal/tests/pytests/util/diff_util/test_diff_util.py +++ b/internal/tests/pytests/util/diff_util/test_diff_util.py @@ -13,7 +13,12 @@ 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] @@ -21,12 +26,12 @@ def create_diff_files(files_a, files_b): dir_b = os.path.join(test_output_dir, f'diff_{unique_id}', 'b') mkdir_p(dir_a) mkdir_p(dir_b) - write_files(dir_a, files_a) - write_files(dir_b, files_b) + write_test_files(dir_a, files_a) + write_test_files(dir_b, files_b) return dir_a, dir_b -def write_files(dirname, files): +def write_test_files(dirname, files): for filename, lines in files.items(): filepath = os.path.join(dirname, filename) if os.path.sep in filename: @@ -38,132 +43,112 @@ def write_files(dirname, files): 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.util -def test_diff_dirs_both_empty(): - a_dir, b_dir = create_diff_files({}, {}) - assert dirs_are_equal(a_dir, b_dir) - shutil.rmtree(os.path.dirname(a_dir)) - - -@pytest.mark.util -def test_diff_dirs_one_empty(): - test_files = {'filename.txt': ['some', 'text']} - for empty_one in ('a', 'b'): - a_files = b_files = {} - if empty_one == 'a': - b_files = test_files - else: - a_files = test_files - a_dir, b_dir = create_diff_files(a_files, b_files) - assert not dirs_are_equal(a_dir, b_dir) - shutil.rmtree(os.path.dirname(a_dir)) - - -@pytest.mark.util -def test_diff_files_both_empty(): - test_files = {'filename.txt': []} - a_dir, b_dir = create_diff_files(test_files, test_files) - assert dirs_are_equal(a_dir, b_dir) - shutil.rmtree(os.path.dirname(a_dir)) - - -@pytest.mark.util -def test_diff_files_one_empty(): - filename = 'filename.txt' - test_content = ['some', 'text'] - for empty_one in ('a', 'b'): - a_files = {filename: []} - b_files = {filename: []} - if empty_one == 'a': - b_files[filename].extend(test_content) - else: - a_files[filename].extend(test_content) - a_dir, b_dir = create_diff_files(a_files, b_files) - assert not dirs_are_equal(a_dir, b_dir) - shutil.rmtree(os.path.dirname(a_dir)) - - -@pytest.mark.util -def test_diff_stat_header_columns(): - filename = 'filename.stat' - a_content = [stat_header, mpr_line_1] - b_content = [f'{stat_header} NEW_COLUMN', mpr_line_1] - a_files = {filename: a_content} - b_files = {filename: b_content} - a_dir, b_dir = create_diff_files(a_files, b_files) - assert not dirs_are_equal(a_dir, b_dir) - shutil.rmtree(os.path.dirname(a_dir)) - - -@pytest.mark.util -def test_diff_stat_number_of_lines(): - filename = 'filename.stat' - a_content = [stat_header, mpr_line_1] - b_content = [stat_header, mpr_line_1, mpr_line_2] - a_files = {filename: a_content} - b_files = {filename: b_content} - a_dir, b_dir = create_diff_files(a_files, b_files) - assert not dirs_are_equal(a_dir, b_dir) - shutil.rmtree(os.path.dirname(a_dir)) - - -@pytest.mark.util -def test_diff_stat_number_of_columns(): - filename = 'filename.stat' - a_content = [stat_header, mpr_line_1] - b_content = [stat_header, f'{mpr_line_1} extra_value'] - a_files = {filename: a_content} - b_files = {filename: b_content} - a_dir, b_dir = create_diff_files(a_files, b_files) - assert not dirs_are_equal(a_dir, b_dir) - shutil.rmtree(os.path.dirname(a_dir)) +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 - -@pytest.mark.util -def test_diff_stat_string(): - filename = 'filename.stat' - a_content = [stat_header, mpr_line_1] - b_content = [stat_header, mpr_line_1.replace('L0', 'Z0')] - a_files = {filename: a_content} - b_files = {filename: b_content} a_dir, b_dir = create_diff_files(a_files, b_files) - assert not dirs_are_equal(a_dir, b_dir) - shutil.rmtree(os.path.dirname(a_dir)) + 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 -@pytest.mark.util -def test_diff_stat_float_default_precision(): - filename = 'filename.stat' - a_content = [stat_header, mpr_line_1] - b_content = [stat_header, mpr_line_1.replace('39.78616', '39.78615')] - a_files = {filename: a_content} - b_files = {filename: b_content} - a_dir, b_dir = create_diff_files(a_files, b_files) - assert not dirs_are_equal(a_dir, b_dir) - shutil.rmtree(os.path.dirname(a_dir)) - - -@pytest.mark.util -def test_diff_stat_float_override_precision(): - filename = 'filename.stat' - ROUNDING_OVERRIDES[filename] = 4 - a_content = [stat_header, mpr_line_1] - b_content = [stat_header, mpr_line_1.replace('39.78616', '39.78615')] - a_files = {filename: a_content} - b_files = {filename: b_content} - a_dir, b_dir = create_diff_files(a_files, b_files) - assert dirs_are_equal(a_dir, b_dir) - shutil.rmtree(os.path.dirname(a_dir)) - - -@pytest.mark.util -def test_diff_stat_out_of_order(): - filename = 'filename.stat' - ROUNDING_OVERRIDES[filename] = 4 - a_content = [stat_header, mpr_line_1, mpr_line_2] - b_content = [stat_header, mpr_line_2, mpr_line_1] - a_files = {filename: a_content} - b_files = {filename: b_content} - a_dir, b_dir = create_diff_files(a_files, b_files) - assert dirs_are_equal(a_dir, b_dir) shutil.rmtree(os.path.dirname(a_dir)) From 59de5c98888d8d2e7dfc68af398dfbe3b8c53743 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 18 Jul 2023 12:00:08 -0600 Subject: [PATCH 36/41] remove file containing pytest groups because they are no longer required to run all of the tests --- .github/actions/run_tests/entrypoint.sh | 2 -- .github/parm/pytest_groups.txt | 8 -------- 2 files changed, 10 deletions(-) delete mode 100644 .github/parm/pytest_groups.txt diff --git a/.github/actions/run_tests/entrypoint.sh b/.github/actions/run_tests/entrypoint.sh index 11de254d32..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 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 From 0982a55261f95437a7ba09b74a438d0aadb902c9 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 18 Jul 2023 12:01:04 -0600 Subject: [PATCH 37/41] add diff marker to be able to run all unit tests except the diff utility tests that have additional dependencies that may not be available on all systems --- internal/tests/pytests/pytest.ini | 1 + internal/tests/pytests/util/diff_util/test_diff_util.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) 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 index 7e1ddca20f..7ac2c837b4 100644 --- a/internal/tests/pytests/util/diff_util/test_diff_util.py +++ b/internal/tests/pytests/util/diff_util/test_diff_util.py @@ -20,6 +20,7 @@ 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') @@ -135,7 +136,7 @@ def write_test_files(dirname, files): None, False), ] ) -@pytest.mark.util +@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: From d3033c6597a6ca9092fd0b55a6ce83610610d4fa Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 18 Jul 2023 12:02:39 -0600 Subject: [PATCH 38/41] per #2244, updated documentation about running unit tests. removed section about running use case tests because they are typically run in GitHub Actions instead of locally. Use case tests can still be run manually by running a use case and writing output to 2 different directories, then comparing the results --- docs/Contributors_Guide/testing.rst | 98 +++++++++++++++++------------ 1 file changed, 57 insertions(+), 41 deletions(-) diff --git a/docs/Contributors_Guide/testing.rst b/docs/Contributors_Guide/testing.rst index f0a4b06f44..c5db882bfd 100644 --- a/docs/Contributors_Guide/testing.rst +++ b/docs/Contributors_Guide/testing.rst @@ -9,20 +9,60 @@ 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 +* **pillow**: Only used if running diff utility tests +* **pdf2image**: 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 + +Code Coverage +^^^^^^^^^^^^^ + +If the *pytest-cov* Python 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 + 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. -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 @@ -48,43 +88,19 @@ 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 -m " or " - - -Use Case Tests --------------- - -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. - -To see which files and directories are only found in one run:: - - diff -r /d1/mccabe/test-use-case-a /d1/mccabe/test-use-case-b | grep Only + pytest internal/tests/pytests -m " or " From c14740497f326f4ba3db2c2aea9219da7f89c16c Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 18 Jul 2023 12:46:46 -0600 Subject: [PATCH 39/41] added info about using metplus_config fixture in unit tests --- docs/Contributors_Guide/testing.rst | 35 +++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/docs/Contributors_Guide/testing.rst b/docs/Contributors_Guide/testing.rst index c5db882bfd..d3d9f385b2 100644 --- a/docs/Contributors_Guide/testing.rst +++ b/docs/Contributors_Guide/testing.rst @@ -36,6 +36,10 @@ permissions, nativate to the METplus directory, then call pytest:: cd METplus pytest internal/tests/pytests +To view verbose test output, add the **-vv** argument:: + + pytest internal/tests/pytests -vv + Code Coverage ^^^^^^^^^^^^^ @@ -104,3 +108,34 @@ Multiple marker groups can be run by using the 'or' keyword:: pytest internal/tests/pytests -m " or " +Writing Unit Tests +^^^^^^^^^^^^^^^^^^ + +metplus_config fixture +"""""""""""""""""""""" + +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. + +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:: + + def test_something(metplus_config) + +then set a variable called **config** using the fixture name:: + + config = metplus_config + +Additional configuration variables can be set by using the set method:: + + config.set('config', key, value) From 10ae204a8b3caeaca325dad68976500841e6a6e2 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 18 Jul 2023 12:48:46 -0600 Subject: [PATCH 40/41] set DO_NOT_RUN_EXE for all tests to prevent the actual commands from running unless explicitly specified --- internal/tests/pytests/minimum_pytest.conf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/tests/pytests/minimum_pytest.conf b/internal/tests/pytests/minimum_pytest.conf index 4c4cf264f3..703338e43c 100644 --- a/internal/tests/pytests/minimum_pytest.conf +++ b/internal/tests/pytests/minimum_pytest.conf @@ -3,6 +3,8 @@ 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_OUTPUT_BASE]} +DO_NOT_RUN_EXE = True + LOG_LEVEL = DEBUG LOG_LEVEL_TERMINAL = WARNING LOG_MET_OUTPUT_TO_METPLUS = no From e2db1cfc086d9d156f7f6dd3ac6739ebcad3ad4b Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 18 Jul 2023 13:18:27 -0600 Subject: [PATCH 41/41] clean up documentation about unit tests --- docs/Contributors_Guide/testing.rst | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/docs/Contributors_Guide/testing.rst b/docs/Contributors_Guide/testing.rst index d3d9f385b2..1229359804 100644 --- a/docs/Contributors_Guide/testing.rst +++ b/docs/Contributors_Guide/testing.rst @@ -22,8 +22,9 @@ 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 -* **pillow**: Only used if running diff utility tests -* **pdf2image**: Only used if running diff utility tests +* **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 ^^^^^^^ @@ -36,6 +37,7 @@ permissions, nativate to the METplus directory, then call pytest:: cd METplus pytest internal/tests/pytests +A report will be output showing which pytest categories failed. To view verbose test output, add the **-vv** argument:: pytest internal/tests/pytests -vv @@ -43,12 +45,13 @@ To view verbose test output, add the **-vv** argument:: Code Coverage ^^^^^^^^^^^^^ -If the *pytest-cov* Python package is installed, the code coverage report can +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 -A report will be output showing which pytest categories failed. +In addition to the pass/fail report, the code coverage information will be +displayed including line numbers that are not covered by any test. Subsetting Tests by Directory ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -88,8 +91,6 @@ 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 internal/tests/pytests -m @@ -98,13 +99,13 @@ To run all tests that do not have a given marker, run:: 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 +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:: +Multiple marker groups can be run by using the *or* keyword:: pytest internal/tests/pytests -m " or " @@ -115,7 +116,7 @@ metplus_config fixture """""""""""""""""""""" Many unit tests utilize a pytest fixture named **metplus_config**. -This is defined in the conftest.py file in internal/tests/pytests. +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