Skip to content

Fix unit tests#120

Merged
lfoppiano merged 1 commit intomainfrom
bugfix/fix-tests
Jan 9, 2026
Merged

Fix unit tests#120
lfoppiano merged 1 commit intomainfrom
bugfix/fix-tests

Conversation

@lfoppiano
Copy link
Owner

@lfoppiano lfoppiano commented Jan 9, 2026

I've merged #119 too quickly.

Summary by CodeRabbit

  • Tests
    • Improved reliability of accessibility tests by adding explicit waits for canvas visibility and enhanced null-safety checks before assertions.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 9, 2026

📝 Walkthrough

Walkthrough

Test file improvements adding explicit waits and None-check assertions to accessibility test functions, ensuring canvas visibility and bounding box availability before computing dimensions on zoom and responsive text sizing scenarios.

Changes

Cohort / File(s) Summary
Test Robustness Improvements
tests/test_accessibility.py
Adds explicit wait for canvas visibility in test_accessibility_zoom_levels_for_visibility. Introduces None-check assertions for canvas bounding_box before accessing width/height properties in both test_accessibility_zoom_levels_for_visibility and test_accessibility_responsive_text_sizing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 With patience we wait, for the canvas to show,
Before measuring bounds, we now surely know,
A None-check for safety, both steady and sound,
These tests are more stable, on solid ground! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Fix unit tests' is vague and generic, failing to specify which tests are being fixed or what the actual fix addresses. Use a more descriptive title that explains the specific issue being fixed, such as 'Fix canvas bounding box assertions in accessibility tests' or 'Add null checks for canvas visibility in accessibility tests'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 02410ef and cb45811.

📒 Files selected for processing (1)
  • tests/test_accessibility.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: build (ubuntu-latest, 3.11, 20)
  • GitHub Check: build (ubuntu-latest, 3.13, 20)
  • GitHub Check: build (ubuntu-latest, 3.12, 18)
  • GitHub Check: build (ubuntu-latest, 3.10, 18)
  • GitHub Check: build (ubuntu-latest, 3.12, 20)
  • GitHub Check: build (ubuntu-latest, 3.10, 20)
  • GitHub Check: build (ubuntu-latest, 3.11, 18)
  • GitHub Check: build (ubuntu-latest, 3.13, 18)
🔇 Additional comments (2)
tests/test_accessibility.py (2)

47-53: LGTM! Excellent fix for test reliability.

The addition of the explicit visibility wait and None-check assertion properly addresses timing issues where bounding_box() might be called before the canvas is fully rendered. This defensive pattern prevents flaky test failures and provides clearer error messages.


128-128: LGTM! Good defensive check with helpful context.

The None-check assertion completes the defensive pattern for this test function (which already had the visibility wait on line 124). The viewport-specific error message is particularly helpful for debugging responsive layout issues.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@lfoppiano lfoppiano merged commit 9511228 into main Jan 9, 2026
34 checks passed
@lfoppiano lfoppiano deleted the bugfix/fix-tests branch January 9, 2026 17:08
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

Comments