Update result analyzer and remaining changes#23
Update result analyzer and remaining changes#23eerxuan wants to merge 1 commit intodocumentdb:mainfrom
Conversation
xgerman
left a comment
There was a problem hiding this comment.
LGTM as a human.
AI felt this could be done:
Issues Found
🟡 Medium — Import at function scope inside print_summary() report_generator.py line ~158: from collections
import Counter is imported inside the function body. This should be a top-level import per PEP 8. It works, but
is inconsistent with the rest of the file.
🟡 Medium — failure_type not set for non-FAIL outcomes analyzer.py line ~340-345: failure_type is only added to
the test detail dict when test_outcome == FAIL. But report_generator.py calls test.get("failure_type", "UNKNOWN")
— this works due to .get() with default, but the field's conditional presence is fragile. Consider always setting
it (e.g., "N/A" for passed/skipped).
🟢 Minor — Trailing whitespace cleanup is noise Many hunks are whitespace-only changes (trailing spaces removed).
Not a problem, but inflates the diff. Could have been a separate commit.
🟢 Minor — extract_failure_tag regex is broad
match = re.search(r'[([A-Z_]+)]', crash_message)
This matches any [UPPER_CASE] bracket pattern. If a crash message happens to contain something like
[SOME_BSON_KEY], it could false-match. Not likely in practice but worth noting.
🟢 Minor — Empty init.py tests/collection/init.py is added with just a docstring. Fine for package
recognition, but if PR #20 already added test files under tests/collection/, this should have been part of that
PR.
Update result_analyzer with improved categorization and reporting. Signed-off-by: Yunxuan Shi <yunxuan@amazon.com>
650a516 to
e515b94
Compare
Update result_analyzer with improved categorization and reporting.
This has dependency on #20.