From e6ccc05eb7d52e0581761dcdf62c4d331abae17f Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Fri, 2 Aug 2024 16:21:18 -0700 Subject: [PATCH 1/6] Fixes imports --- CIME/hist_utils.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/CIME/hist_utils.py b/CIME/hist_utils.py index b67301c2824..9730e350b7a 100644 --- a/CIME/hist_utils.py +++ b/CIME/hist_utils.py @@ -1,6 +1,11 @@ """ Functions for actions pertaining to history files. """ +import logging +import os +import re +import filecmp + from CIME.XML.standard_module_setup import * from CIME.config import Config from CIME.test_status import TEST_NO_BASELINES_COMMENT, TEST_STATUS_FILENAME @@ -11,8 +16,7 @@ SharedArea, parse_test_name, ) - -import logging, os, re, filecmp +from CIME.utils import CIMEError logger = logging.getLogger(__name__) From e235c83c8287603531d27eed120dd803f29c9fbb Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Fri, 2 Aug 2024 16:22:11 -0700 Subject: [PATCH 2/6] Fixes handling errors raised from invoking cprnc --- CIME/hist_utils.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/CIME/hist_utils.py b/CIME/hist_utils.py index 9730e350b7a..a5308b66df1 100644 --- a/CIME/hist_utils.py +++ b/CIME/hist_utils.py @@ -339,6 +339,10 @@ def _compare_hists( if not ".nc" in hist1: logger.info("Ignoring non-netcdf file {}".format(hist1)) continue + + success = False + cprnc_log_file = None + try: success, cprnc_log_file, cprnc_comment = cprnc( model, @@ -350,10 +354,10 @@ def _compare_hists( outfile_suffix=outfile_suffix, ignore_fieldlist_diffs=ignore_fieldlist_diffs, ) - except: - cprnc_comment = "CPRNC executable not found" - cprnc_log_file = None - success = False + except CIMEError as e: + cprnc_comment = str(e) + except Exception as e: + cprnc_comment = f"Unknown CRPRC error: {e!s}" if success: comments += " {} matched {}\n".format(hist1, hist2) @@ -512,7 +516,10 @@ def cprnc( comment = CPRNC_FIELDLISTS_DIFFER elif "files seem to be IDENTICAL" in out: files_match = True + elif "Failed to open file" in out: + raise CIMEError("Failed to open file") else: + # TODO convert to CIMEError expect( False, "Did not find an expected summary string in cprnc output:\n{}".format( From e33bedee161b8ba844b7109420799cb0316cba52 Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Fri, 2 Aug 2024 16:52:14 -0700 Subject: [PATCH 3/6] Updates get_ts_synopsis, handle additional output --- CIME/hist_utils.py | 83 ++++++++++++++++++++++++---------------------- 1 file changed, 44 insertions(+), 39 deletions(-) diff --git a/CIME/hist_utils.py b/CIME/hist_utils.py index a5308b66df1..905f08e1ef3 100644 --- a/CIME/hist_utils.py +++ b/CIME/hist_utils.py @@ -36,6 +36,7 @@ NO_ORIGINAL = "had no original counterpart" FIELDLISTS_DIFFER = "had a different field list from" DIFF_COMMENT = "did NOT match" +FAILED_OPEN = "Failed to open file" # COMPARISON_COMMENT_OPTIONS should include all of the above: these are any of the special # comment strings that describe the reason for a comparison failure COMPARISON_COMMENT_OPTIONS = set( @@ -733,44 +734,48 @@ def get_ts_synopsis(comments): 'DIFF' >>> get_ts_synopsis('File foo had no compare counterpart in bar with suffix baz\n File foo had no original counterpart in bar with suffix baz\n') 'DIFF' + >>> get_ts_synopsis('file1=\nfile2=\nFailed to open file\n') + 'ERROR failed to open files' + >>> get_ts_synopsis('file1=\nfile2=\nSome other error\n') + 'Could not interpret CPRNC output' """ - if not comments: - return "" - elif "\n" not in comments.strip(): - return comments.strip() + comments = comments.strip() + + if comments == "" or "\n" not in comments: + return comments + + fieldlist_differences = re.search(FIELDLISTS_DIFFER, comments) is not None + baseline_fail = re.search(NO_COMPARE, comments) is not None + real_fail = [ + re.search(x, comments) is not None for x in COMPARISON_FAILURE_COMMENT_OPTIONS + ] + open_fail = re.search(FAILED_OPEN, comments) is not None + + if any(real_fail): + # If there are any real differences, we just report that: we assume that the + # user cares much more about those real differences than fieldlist or bfail + # issues, and we don't want to complicate the matter by trying to report all + # issues in this case. + synopsis = "DIFF" + elif fieldlist_differences and baseline_fail: + # It's not clear which of these (if either) the user would care more + # about, so we report both. We deliberately avoid printing the keywords + # 'FIELDLIST' or TEST_NO_BASELINES_COMMENT (i.e., 'BFAIL'): if we printed + # those, then (e.g.) a 'grep -v FIELDLIST' (which the user might do if + # (s)he was expecting fieldlist differences) would also filter out this + # line, which we don't want. + synopsis = ( + "MULTIPLE ISSUES: field lists differ and some baseline files were missing" + ) + elif fieldlist_differences: + synopsis = "FIELDLIST field lists differ (otherwise bit-for-bit)" + elif baseline_fail: + synopsis = "ERROR {} some baseline files were missing".format( + TEST_NO_BASELINES_COMMENT + ) + elif open_fail: + synopsis = "ERROR failed to open files" else: - has_fieldlist_differences = False - has_bfails = False - has_real_fails = False - for line in comments.splitlines(): - if FIELDLISTS_DIFFER in line: - has_fieldlist_differences = True - if NO_COMPARE in line: - has_bfails = True - for comparison_failure_comment in COMPARISON_FAILURE_COMMENT_OPTIONS: - if comparison_failure_comment in line: - has_real_fails = True - - if has_real_fails: - # If there are any real differences, we just report that: we assume that the - # user cares much more about those real differences than fieldlist or bfail - # issues, and we don't want to complicate the matter by trying to report all - # issues in this case. - return "DIFF" - else: - if has_fieldlist_differences and has_bfails: - # It's not clear which of these (if either) the user would care more - # about, so we report both. We deliberately avoid printing the keywords - # 'FIELDLIST' or TEST_NO_BASELINES_COMMENT (i.e., 'BFAIL'): if we printed - # those, then (e.g.) a 'grep -v FIELDLIST' (which the user might do if - # (s)he was expecting fieldlist differences) would also filter out this - # line, which we don't want. - return "MULTIPLE ISSUES: field lists differ and some baseline files were missing" - elif has_fieldlist_differences: - return "FIELDLIST field lists differ (otherwise bit-for-bit)" - elif has_bfails: - return "ERROR {} some baseline files were missing".format( - TEST_NO_BASELINES_COMMENT - ) - else: - return "" + synopsis = "Could not interpret CPRNC output" + + return synopsis From d674fa2f949709cccc33524b9321c279cd4091f8 Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Fri, 2 Aug 2024 17:07:04 -0700 Subject: [PATCH 4/6] Fixes inspecting cause of baseline fail to set overall status --- CIME/test_status.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/CIME/test_status.py b/CIME/test_status.py index 5f32486ab51..da818b2aca3 100644 --- a/CIME/test_status.py +++ b/CIME/test_status.py @@ -467,7 +467,14 @@ def _get_overall_status_based_on_phases( elif phase in [BASELINE_PHASE, THROUGHPUT_PHASE, MEMCOMP_PHASE]: if rv in [NAMELIST_FAIL_STATUS, TEST_PASS_STATUS]: phase_responsible_for_status = phase - rv = TEST_DIFF_STATUS + # need to further inspect message to determine + # phase status + if "DIFF" in data[1]: + rv = TEST_DIFF_STATUS + elif "ERROR" in data[1]: + rv = TEST_FAIL_STATUS + else: + rv = TEST_DIFF_STATUS else: pass # a DIFF does not trump a FAIL From 4791bd043f0dfee4b923b85f7e9cad8cb3040629 Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Fri, 2 Aug 2024 17:13:44 -0700 Subject: [PATCH 5/6] Adds additional context --- CIME/hist_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CIME/hist_utils.py b/CIME/hist_utils.py index 905f08e1ef3..16a0ac800bc 100644 --- a/CIME/hist_utils.py +++ b/CIME/hist_utils.py @@ -774,7 +774,7 @@ def get_ts_synopsis(comments): TEST_NO_BASELINES_COMMENT ) elif open_fail: - synopsis = "ERROR failed to open files" + synopsis = "ERROR CPRNC failed to open files" else: synopsis = "Could not interpret CPRNC output" From 3a3fc3459d40a1d29f08097f8fcca9e05cd1a23d Mon Sep 17 00:00:00 2001 From: Jason Boutte Date: Mon, 5 Aug 2024 09:38:29 -0700 Subject: [PATCH 6/6] Fixes doctest --- CIME/hist_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CIME/hist_utils.py b/CIME/hist_utils.py index 16a0ac800bc..ea286c4454a 100644 --- a/CIME/hist_utils.py +++ b/CIME/hist_utils.py @@ -735,7 +735,7 @@ def get_ts_synopsis(comments): >>> get_ts_synopsis('File foo had no compare counterpart in bar with suffix baz\n File foo had no original counterpart in bar with suffix baz\n') 'DIFF' >>> get_ts_synopsis('file1=\nfile2=\nFailed to open file\n') - 'ERROR failed to open files' + 'ERROR CPRNC failed to open files' >>> get_ts_synopsis('file1=\nfile2=\nSome other error\n') 'Could not interpret CPRNC output' """