Fix comparison bugs and modernize#15
Merged
Merged
Conversation
Bug fixes: - common.py: use filecmp.cmp(shallow=False) so files with matching size+mtime but different content are no longer falsely reported as identical, which would skip the render diff entirely. - common.py: --driver none no longer crashes on byte-differing HTML. The no-browser case now reports such files as different (they already differ at the byte level and cannot be rendered) instead of raising a confusing TypeError from get_browser() called without a driver. - html_render_diff.py: add type=int to --max-width/--max-height, which otherwise arrived as strings and failed the int type check in get_browser(). - pyproject.toml: bump requires-python to >=3.10; the code uses PEP 604 unions (str | None) as runtime-evaluated annotations, so it cannot import on 3.7-3.9. Server: - compare_output_server.py: serialize access to the single shared Selenium browser with a lock; Flask serves /image_diff from a thread pool and webdrivers are not thread-safe. - compare_output_server.py: wire the bind host through a --host argument (default 0.0.0.0) instead of hardcoding it. Cleanups: - Simplify a redundant boolean and tidy_output.py's file/dir loops. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SujCMUVctCunqAft7ttLFd
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug fixes, a server threading fix, and small cleanups found while auditing the codebase. All 3 tests pass (including the Chrome/Firefox render tests), and each fix was verified end-to-end.
Bugs
common.py):filecmp.cmpdefaulted toshallow=True, comparing onlyos.stat(size + mtime) rather than content. Two differently-rendered files with matching stat signatures compared as equal, silently skipping the render diff that is the whole point of the tool. Nowshallow=False. Reproduced and confirmed fixed.--driver nonecrash (common.py): with no browser, a byte-differing HTML file routed intocompare_html, whose fallbackget_browser()was called with nodriverargument (required) → confusingTypeError, contradicting the documented "compares only JSON and byte-identical files". The no-browser case now reports such files as different (they already differ at the byte level and can't be rendered). The broken auto-create fallback is also repaired for direct callers.--max-width/--max-heightcrashed when passed (html_render_diff.py): missingtype=int, so values arrived as strings and tripped theisinstance(..., int)guard inget_browser. Defaults worked; only explicit use was broken.pyproject.toml): declared>=3.7, but the code uses PEP 604 unions (str | None) as runtime-evaluated annotations, which need ≥3.10 — the package wouldn't import on 3.7–3.9. Bumped to>=3.10.Server
compare_output_server.py): the singleConfig.browseris used from Flask's threaded request handlers in/image_diff; Selenium drivers aren't thread-safe, so concurrent diff requests could interleave on one driver. Access is now serialized with a lock.--hostwired through args: the bind host was hardcoded to0.0.0.0; it's now a--hostargument (default kept at0.0.0.0so the Docker use-case still works).Cleanups
True if … else Falseand the awkward name→path reconstruction loops intidy_output.py.🤖 Generated with Claude Code