Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes handling cprnc output #4653

Merged
merged 6 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 61 additions & 45 deletions CIME/hist_utils.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -11,8 +16,7 @@
SharedArea,
parse_test_name,
)

import logging, os, re, filecmp
from CIME.utils import CIMEError

logger = logging.getLogger(__name__)

Expand All @@ -32,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(
Expand Down Expand Up @@ -335,6 +340,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,
Expand All @@ -346,10 +355,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)
Expand Down Expand Up @@ -508,7 +517,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(
Expand Down Expand Up @@ -722,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 CPRNC 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 CPRNC 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
9 changes: 8 additions & 1 deletion CIME/test_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading