Fix cli.py --html traceback crash and add error-path test covera - #277
Conversation
|
@evanjain-dot is attempting to deploy a commit to the yashkewlani2020-gmailcom's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@yakew7 @ahmdkaml @Shreyash0712 - new PR to review. |
|
Looks good to me as well. @yakew7 Minimal changes and solves the bug. Though my agent did raise a flag - Currently, to_html(result) is evaluated while the file is already open for writing. If to_html(result) were to ever throw an unexpected exception (e.g., a ValueError or TypeError during formatting), the script would crash, but it would leave behind an empty 0-byte file on the user's disk since open() already succeeded. Generating the HTML string right before opening the file avoids this edge case entirely: (The exact same logic applies to the compare command section below it with compare_to_html(result)). Again, this is a super minor edge case since the formatting functions shouldn't be crashing anyway, but it's a good pattern to keep I/O operations as isolated as possible. |
to_html()/compare_to_html() previously ran inside the open() context, so a formatting exception (not just an OSError on the write itself) would leave a truncated 0-byte report file behind. Generating the string first keeps I/O and computation isolated, per Shreyash0712's review comment on PR #277. Co-authored-by: Shreyash0712 <123396545+Shreyash0712@users.noreply.github.com>
|
Good catch @Shreyash0712 - implemented in 449ab5a: to_html()/compare_to_html() now run before the file is opened, so a formatting exception can't leave a truncated 0-byte report behind. |
Credits evanjain-dot's #277 (--html traceback fix + CLI test coverage) and #279 (matplotlib docstring fix), propcgamer20-png's #278 (favicon-parsing test coverage), and the two review-driven follow-ups (Shreyash0712 on #277, ahmdkaml on #278). Also logs the pr-review-ping.yml path-scoping fix, which wasn't recorded yet.
html previously crashed with a raw traceback if its output
directory didn't exist; now catches OSError like every other
error path in this file (closes #268).
Added tests/test_cli.py coverage for every branch make coverage
flagged as untested: --map/--cross/--reference/--proxy-hints
error handling and both --html write branches (closes #269).