Clean up hover test.#9826
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the HoverCard component and its associated tests by exposing internal constants like hoverMargin and hoverDelay for testing purposes. It also cleans up hover_positioning_test.dart by replacing hardcoded dimensions and durations with named constants. A review comment identifies a remaining magic number in the tests that should be calculated relative to the window height and margin to improve robustness and adhere to the style guide.
| // Clamps strictly at y = 274.0 because of dynamic height containing title/divider. | ||
| expect(positionWithTitle.dy, equals(274.0)); |
There was a problem hiding this comment.
[CONCERN] This test uses a hardcoded magic number 274.0 for the expected position. Since the PR aims to clean up the test and already introduces constants like _windowHeight and makes hoverMargin public, this expectation should be calculated relative to those constants. This makes the test more robust against changes to the window size or margin values. Additionally, using the actual height of the rendered box ensures the test correctly verifies that the bottom of the card is positioned at the expected margin from the window edge.
expect(
positionWithTitle.dy + renderBoxWithTitle.size.height,
equals(_windowHeight - hoverMargin),
);References
- Strict avoidance of raw values in UI (use named constants) and general code quality (DRY, Meaningful Naming). (link)
…branch 'master' of github.com:flutter/devtools into tooltipfollowup
A follow up to #9823 to clean up the test.