Skip to content

Mock webbrowser.open() in test_dotfile to stop popping up a PDF - #9

Merged
petercorke merged 2 commits into
mainfrom
test/no-popup-in-tests
Sep 3, 2026
Merged

petercorke merged 2 commits into
mainfrom
test/no-popup-in-tests

Conversation

@petercorke

Copy link
Copy Markdown
Owner

Summary

  • showgraph() calls webbrowser.open() on the rendered PDF, which was launching a real PDF viewer/browser window every test run -- purely incidental, since the test made no assertion on the result at all.
  • Mocks webbrowser.open() so the test runs headless, and replaces the "doesn't crash" smoke test with a real assertion: the file webbrowser.open() was called with actually exists and starts with the PDF magic bytes (%PDF-), confirming dot's rendering pipeline genuinely produced a valid PDF -- a stronger check than before, not a weaker one.

Test plan

  • test_dotfile passes headless, no popup
  • 44/44 tests pass

petercorke and others added 2 commits September 3, 2026 15:34
showgraph() calls webbrowser.open() on the rendered PDF, which was
launching a real PDF viewer/browser window every test run -- purely
incidental, since the test made no assertion on the result at all.

Mocks webbrowser.open() so the test runs headless, and replaces the
"doesn't crash" smoke test with a real assertion: the file
webbrowser.open() was called with actually exists and starts with the
PDF magic bytes (%PDF-), confirming dot's rendering pipeline genuinely
produced a valid PDF -- a stronger check than before, not a weaker one.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
test_dotfile() exercises the real dot->PDF rendering path via
showgraph(), but only the docs job installed graphviz -- unittest
and codecov didn't, so `dot` was silently missing there. The previous
version of this test had no assertion on the result, so this went
unnoticed; the new assertion (mock_open.assert_called_once(), added
in this same PR) correctly caught it: CI failed with "dot: not
found", showgraph()'s subprocess call returned nonzero, and
webbrowser.open() was consequently never reached.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.33%. Comparing base (6c24cfe) to head (ea2336a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main       #9      +/-   ##
==========================================
+ Coverage   85.20%   85.33%   +0.13%     
==========================================
  Files           2        2              
  Lines         750      750              
==========================================
+ Hits          639      640       +1     
+ Misses        111      110       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@petercorke
petercorke merged commit 285809e into main Sep 3, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant