Skip to content

Commit 78d3fc0

Browse files
takishidatakishida
authored
273 exception handling for check_wrapper and print_result (#284)
* fix: Exception handling in print function * fix: Do AciResult.writeResult() in finally
1 parent b2ae8f5 commit 78d3fc0

2 files changed

Lines changed: 74 additions & 45 deletions

File tree

‎aci-preupgrade-validation-script.py‎

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,28 +1050,29 @@ def wrapper(index, total_checks, *args, **kwargs):
10501050
synth.writeResult()
10511051
return None
10521052

1053-
# Print `[Check 1/81] <title>...`
1054-
print_title(check_title, index, total_checks)
1055-
10561053
try:
1054+
# Print `[Check 1/81] <title>...`
1055+
print_title(check_title, index, total_checks)
1056+
10571057
# Run check, expecting it to return a `Result` object
10581058
r = check_func(*args, **kwargs)
1059+
1060+
# Print `[Check 1/81] <title>... <msg> <result>\n<failure details>`
1061+
print_result(title=check_title, **r.as_dict())
10591062
except Exception as e:
1060-
r = Result(result=ERROR, msg='Unexpected Error: {}'.format(e))
10611063
log.exception(e)
1062-
1063-
# Print `[Check 1/81] <title>... <result> + <failure details>`
1064-
print_result(title=check_title, **r.as_dict())
1065-
1066-
# Write results in JSON
1067-
# Using `wrapper.__name__` instead of `check_func.__name` because
1068-
# both show the original check func name and `wrapper.__name__` can
1069-
# be dynamically changed inside each check func if needed. (mainly
1070-
# for test or debugging)
1071-
synth = AciResult(wrapper.__name__, check_title, "")
1072-
synth.updateWithResults(**r.as_dict_for_json_result())
1073-
synth.writeResult()
1074-
return r.result
1064+
r = Result(result=ERROR, msg='Unexpected Error: {}'.format(e))
1065+
print_result(title=check_title, **r.as_dict())
1066+
finally:
1067+
# Write results in JSON
1068+
# Using `wrapper.__name__` instead of `check_func.__name` because
1069+
# both show the original check func name and `wrapper.__name__` can
1070+
# be dynamically changed inside each check func if needed. (mainly
1071+
# for test or debugging)
1072+
synth = AciResult(wrapper.__name__, check_title, "")
1073+
synth.updateWithResults(**r.as_dict_for_json_result())
1074+
synth.writeResult()
1075+
return r.result
10751076
return wrapper
10761077
return decorator
10771078

@@ -1215,8 +1216,19 @@ def print_result(title, result, msg='',
12151216
recommended_action='',
12161217
doc_url='',
12171218
adjust_title=False):
1218-
padding = 120 - len(title) - len(msg)
1219-
if adjust_title: padding += len(title) + 18
1219+
FULL_LEN = 138 # length of `[Check XX/YY] <title>... <msg> --padding-- <RESULT>`
1220+
CHECK_LEN = 18 # length of `[Check XX/YY] ... `
1221+
padding = FULL_LEN - CHECK_LEN - len(title) - len(msg)
1222+
if adjust_title:
1223+
# adjust padding when the result is on the second line.
1224+
# 1st: `[Check XX/YY] <title>... `
1225+
# 2nd: ` <msg> --padding-- <RESULT>`
1226+
padding += len(title) + CHECK_LEN
1227+
if padding < len(result):
1228+
# when `msg` is too long (ex. unknown exception), `padding` may get shorter
1229+
# than what it's padding (`result`), or worse, may get negative.
1230+
# In such a case, keep one whitespace padding even if the full length gets longer.
1231+
padding = len(result) + 1
12201232
output = '{}{:>{}}'.format(msg, result, padding)
12211233
if data:
12221234
data.sort()
@@ -5533,6 +5545,13 @@ def run_checks(checks, inputs):
55335545
except KeyboardInterrupt:
55345546
prints('\n\n!!! KeyboardInterrupt !!!\n')
55355547
break
5548+
except Exception as e:
5549+
prints('')
5550+
err = 'Wrapper Error: %s' % e
5551+
print_title(err)
5552+
print_result(title=err, result=ERROR)
5553+
summary[ERROR] += 1
5554+
logging.exception(e)
55365555

55375556
prints('\n=== Summary Result ===\n')
55385557
res = max(summary_headers, key=len)

‎tests/test_run_checks.py‎

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,17 @@
1111
check_wrapper = script.check_wrapper
1212

1313

14+
# 120 = length of `<title> + <msg> --padding-- <RESULT>` in `[Check XX/YY] <title>... <msg> --padding-- <RESULT>`
1415
ERROR_REASON = "This is a test exception to result in `script.ERROR`."
16+
ERROR_REASON_LONG = "This is a looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong test exception to result in `script.ERROR`." # > 120 char
1517

1618

17-
def check_builder(func_name, title, result, others):
19+
def check_builder(func_name, title, result, err_msg, others):
1820
@check_wrapper(check_title=title)
1921
def _check(**kwargs):
2022
_check.__name__ = func_name # Set the function name for the check
2123
if result == script.ERROR:
22-
raise Exception(ERROR_REASON)
24+
raise Exception(err_msg)
2325
else:
2426
return Result(result=result, **others)
2527
return _check
@@ -50,24 +52,30 @@ def _check(**kwargs):
5052
"msg": "test msg",
5153
}
5254

55+
fake_data_only_long_msg = {
56+
"msg": "looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong test msg", # > 120 char
57+
}
58+
5359
fake_checks_meta = [
54-
("fake_check1", "Test Check 1", script.PASS, {}),
55-
("fake_check2", "Test Check 2", script.FAIL_O, fake_data_full),
56-
("fake_check3", "Test Check 3", script.FAIL_UF, fake_data_no_msg_no_unform),
57-
("fake_check4", "Test Check 4", script.MANUAL, fake_data_only_msg),
58-
("fake_check5", "Test Check 5", script.POST, fake_data_only_msg),
59-
("fake_check6", "Test Check 6", script.NA, fake_data_only_msg),
60-
("fake_check7", "Test Check 7", script.ERROR, fake_data_error),
61-
("fake_check8", "Test Check 8", script.PASS, fake_data_only_msg),
60+
("fake_check1", "Test Check 1", script.PASS, "", {}),
61+
("fake_check2", "Test Check 2", script.FAIL_O, "", fake_data_full),
62+
("fake_check3", "Test Check 3", script.FAIL_UF, "", fake_data_no_msg_no_unform),
63+
("fake_check4", "Test Check 4", script.MANUAL, "", fake_data_only_msg),
64+
("fake_check5", "Test Check 5", script.POST, "", fake_data_only_msg),
65+
("fake_check6", "Test Check 6", script.NA, "", fake_data_only_msg),
66+
("fake_check7", "Test Check 7", script.ERROR, ERROR_REASON, fake_data_error),
67+
("fake_check8", "Test Check 8", script.PASS, "", fake_data_only_msg),
68+
("fake_check9", "Test Check 9", script.ERROR, ERROR_REASON_LONG, fake_data_error),
69+
("fake_check10", "Test Check 10", script.NA, "", fake_data_only_long_msg),
6270
]
6371

6472
fake_checks = [
65-
check_builder(func_name, title, result, others)
66-
for func_name, title, result, others in fake_checks_meta
73+
check_builder(func_name, title, result, err_msg, others)
74+
for func_name, title, result, err_msg, others in fake_checks_meta
6775
]
6876

6977
fake_result_filenames = [
70-
"{}.json".format(func_name) for func_name, _, _, _ in fake_checks_meta
78+
"{}.json".format(func_name) for func_name, _, _, _, _ in fake_checks_meta
7179
]
7280

7381
fake_inputs = {
@@ -88,8 +96,8 @@ def test_run_checks(capsys, caplog):
8896
assert (
8997
captured.out
9098
== """\
91-
[Check 1/8] Test Check 1... PASS
92-
[Check 2/8] Test Check 2... test msg FAIL - OUTAGE WARNING!!
99+
[Check 1/10] Test Check 1... PASS
100+
[Check 2/10] Test Check 2... test msg FAIL - OUTAGE WARNING!!
93101
H1 H2 H3
94102
-- -- --
95103
Data1 Data2 Data3
@@ -105,7 +113,7 @@ def test_run_checks(capsys, caplog):
105113
Reference Document: https://fake_doc_url.local/path1/#section1
106114
107115
108-
[Check 3/8] Test Check 3... FAIL - UPGRADE FAILURE!!
116+
[Check 3/10] Test Check 3... FAIL - UPGRADE FAILURE!!
109117
H1 H2 H3
110118
-- -- --
111119
Data1 Data2 Data3
@@ -116,11 +124,13 @@ def test_run_checks(capsys, caplog):
116124
Reference Document: https://fake_doc_url.local/path1/#section1
117125
118126
119-
[Check 4/8] Test Check 4... test msg MANUAL CHECK REQUIRED
120-
[Check 5/8] Test Check 5... test msg POST UPGRADE CHECK REQUIRED
121-
[Check 6/8] Test Check 6... test msg N/A
122-
[Check 7/8] Test Check 7... Unexpected Error: This is a test exception to result in `script.ERROR`. ERROR !!
123-
[Check 8/8] Test Check 8... test msg PASS
127+
[Check 4/10] Test Check 4... test msg MANUAL CHECK REQUIRED
128+
[Check 5/10] Test Check 5... test msg POST UPGRADE CHECK REQUIRED
129+
[Check 6/10] Test Check 6... test msg N/A
130+
[Check 7/10] Test Check 7... Unexpected Error: This is a test exception to result in `script.ERROR`. ERROR !!
131+
[Check 8/10] Test Check 8... test msg PASS
132+
[Check 9/10] Test Check 9... Unexpected Error: This is a looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong test exception to result in `script.ERROR`. ERROR !!
133+
[Check 10/10] Test Check 10... looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong test msg N/A
124134
125135
=== Summary Result ===
126136
@@ -129,9 +139,9 @@ def test_run_checks(capsys, caplog):
129139
FAIL - UPGRADE FAILURE!! : 1
130140
MANUAL CHECK REQUIRED : 1
131141
POST UPGRADE CHECK REQUIRED : 1
132-
N/A : 1
133-
ERROR !! : 1
134-
TOTAL : 8
142+
N/A : 2
143+
ERROR !! : 2
144+
TOTAL : 10
135145
""" # noqa: W291
136146
)
137147

@@ -142,12 +152,12 @@ def test_run_checks(capsys, caplog):
142152
with open(os.path.join(JSON_DIR, json_file)) as f:
143153
data = json.load(f)
144154

145-
for func_name, title, result, others, in fake_checks_meta:
155+
for func_name, title, result, err_msg, others, in fake_checks_meta:
146156
if data["ruleId"] == func_name:
147157
assert data["name"] == title
148158
# reason
149159
if result == script.ERROR:
150-
assert data["reason"].endswith(ERROR_REASON)
160+
assert data["reason"].endswith(err_msg)
151161
elif others.get("unformatted_data"):
152162
assert data["reason"] == others.get("msg", "") + (
153163
"\n"

0 commit comments

Comments
 (0)