Skip to content

Clean up hover test. #9826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kenzieschmoll wants to merge 1 commit into
base: master
Choose a base branch
from
+98 −89
Open

Clean up hover test. #9826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 18 additions & 16 deletions packages/devtools_app/lib/src/shared/ui/hover.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ int _hoverIndexFor(double dx, TextSpan line) {
const _hoverYOffset = 10.0;

/// Minimum distance from the side of screen to show tooltip
const _hoverMargin = 16.0;
@visibleForTesting
const hoverMargin = 16.0;

/// Defines how a [HoverCardTooltip] is positioned
enum HoverCardPosition {
Expand Down Expand Up @@ -257,18 +258,18 @@ class HoverCard {
final localPosition = overlayBox.globalToLocal(event.position);

final maxX = math.max(
_hoverMargin,
overlaySize.width - _hoverMargin - width,
hoverMargin,
overlaySize.width - hoverMargin - width,
);
final x = (localPosition.dx - (width / 2.0)).clamp(_hoverMargin, maxX);
final x = (localPosition.dx - (width / 2.0)).clamp(hoverMargin, maxX);

final maxY = math.max(
_hoverMargin,
hoverMargin,
overlaySize.height -
_hoverMargin -
hoverMargin -
_totalMaxHoverCardHeight(hasTitle: title != null),
);
final y = (localPosition.dy + _hoverYOffset).clamp(_hoverMargin, maxY);
final y = (localPosition.dy + _hoverYOffset).clamp(hoverMargin, maxY);

return Offset(x, y);
}
Expand Down Expand Up @@ -381,7 +382,8 @@ class HoverCardTooltip extends StatefulWidget {
}) : asyncGenerateHoverCardData = null,
asyncTimeout = null;

static const _hoverDelay = Duration(milliseconds: 500);
@visibleForTesting
static const hoverDelay = Duration(milliseconds: 500);
static const defaultHoverWidth = 450.0;

/// Whether the tooltip is currently enabled.
Expand Down Expand Up @@ -420,7 +422,7 @@ class _HoverCardTooltipState extends State<HoverCardTooltip> {

void _onHoverExit() {
_showTimer?.cancel();
_removeTimer = Timer(HoverCardTooltip._hoverDelay, () {
_removeTimer = Timer(HoverCardTooltip.hoverDelay, () {
if (_currentHoverCard != null) {
_hoverCardController.maybeRemoveHoverCard(_currentHoverCard!);
}
Expand Down Expand Up @@ -448,7 +450,7 @@ class _HoverCardTooltipState extends State<HoverCardTooltip> {
final generateHoverCardData = widget.generateHoverCardData;
final asyncTimeout = widget.asyncTimeout;

_showTimer = Timer(HoverCardTooltip._hoverDelay, () {
_showTimer = Timer(HoverCardTooltip.hoverDelay, () {
if (asyncGenerateHoverCardData != null) {
assert(generateHoverCardData == null);
_showAsyncHoverCard(
Expand Down Expand Up @@ -596,13 +598,13 @@ class _HoverCardTooltipState extends State<HoverCardTooltip> {
final box = context.findRenderObject() as RenderBox;

final maxX = math.max(
_hoverMargin,
overlayBox.size.width - _hoverMargin - width,
hoverMargin,
overlayBox.size.width - hoverMargin - width,
);
final maxY = math.max(
_hoverMargin,
hoverMargin,
overlayBox.size.height -
_hoverMargin -
hoverMargin -
_totalMaxHoverCardHeight(hasTitle: title != null),
);

Expand All @@ -612,8 +614,8 @@ class _HoverCardTooltipState extends State<HoverCardTooltip> {
);

return Offset(
offset.dx.clamp(_hoverMargin, maxX),
offset.dy.clamp(_hoverMargin, maxY),
offset.dx.clamp(hoverMargin, maxX),
offset.dy.clamp(hoverMargin, maxY),
);
}

Expand Down
35 changes: 21 additions & 14 deletions packages/devtools_app/test/service/timeline_streams_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,30 @@ void main() {
await env.tearDownEnvironment(force: true);
});

test('timeline streams initialized on vm service opened', () async {
await env.setupEnvironment();
test(
'timeline streams initialized on vm service opened',
() async {
await env.setupEnvironment();

// Await a short delay to make sure the timelineStreamManager is done
// initializing.
await delay();
// Await a short delay to make sure the timelineStreamManager is done
// initializing.
await delay();

expect(serviceConnection.serviceManager.service, equals(env.service));
expect(serviceConnection.timelineStreamManager, isNotNull);
expect(serviceConnection.timelineStreamManager.basicStreams, isNotEmpty);
expect(
serviceConnection.timelineStreamManager.advancedStreams,
isNotEmpty,
);
expect(serviceConnection.serviceManager.service, equals(env.service));
expect(serviceConnection.timelineStreamManager, isNotNull);
expect(
serviceConnection.timelineStreamManager.basicStreams,
isNotEmpty,
);
expect(
serviceConnection.timelineStreamManager.advancedStreams,
isNotEmpty,
);

await env.tearDownEnvironment();
}, timeout: const Timeout.factor(4));
await env.tearDownEnvironment();
},
timeout: const Timeout.factor(4),
);

test('notifies on stream change', () async {
await env.setupEnvironment();
Expand Down
118 changes: 59 additions & 59 deletions packages/devtools_app/test/shared/ui/hover_positioning_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import 'package:devtools_test/helpers.dart';
import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:provider/provider.dart';

const _windowWidth = 800.0;
const _windowHeight = 600.0;
const _windowSize = Size(_windowWidth, _windowHeight);

void main() {
Future<void> pumpHoverCardTooltip(
Expand Down Expand Up @@ -40,13 +43,13 @@ void main() {
await gesture.addPointer(location: Offset.zero);
final center = tester.getCenter(find.text('Hover Me'));
await gesture.moveTo(center);
await tester.pump(const Duration(milliseconds: 500));
await tester.pump(HoverCardTooltip.hoverDelay);
await tester.pumpAndSettle();
}

testWidgetsWithWindowSize(
'HoverCard at the bottom of the window should not overflow',
const Size(800, 600),
_windowSize,
(WidgetTester tester) async {
// Use a title to increase the height beyond the base content height.
await pumpHoverCardTooltip(
Expand All @@ -66,14 +69,16 @@ void main() {
final position = renderBox.localToGlobal(Offset.zero);
final size = renderBox.size;

// _hoverMargin = 16.0
expect(position.dy + size.height, lessThanOrEqualTo(600.0 - 16.0));
expect(
position.dy + size.height,
lessThanOrEqualTo(_windowHeight - hoverMargin),
);
},
);

testWidgetsWithWindowSize(
'HoverCard at the right of the window should not overflow',
const Size(800, 600),
_windowSize,
(WidgetTester tester) async {
await pumpHoverCardTooltip(tester, alignment: Alignment.centerRight);

Expand All @@ -88,8 +93,10 @@ void main() {
final position = renderBox.localToGlobal(Offset.zero);
final size = renderBox.size;

// _hoverMargin = 16.0
expect(position.dx + size.width, lessThanOrEqualTo(800.0 - 16.0));
expect(
position.dx + size.width,
lessThanOrEqualTo(_windowWidth - hoverMargin),
);
},
);

Expand All @@ -110,38 +117,36 @@ void main() {
},
);

testWidgetsWithWindowSize(
'HoverCard height clamping with title',
const Size(800, 600),
(WidgetTester tester) async {
await pumpHoverCardTooltip(
tester,
alignment: Alignment.bottomCenter,
title: 'An Important Title',
);
testWidgetsWithWindowSize('HoverCard height clamping with title', _windowSize, (
WidgetTester tester,
) async {
await pumpHoverCardTooltip(
tester,
alignment: Alignment.bottomCenter,
title: 'An Important Title',
);

final hoverContentFinderWithTitle = find.text('Hover Content');
expect(hoverContentFinderWithTitle, findsOneWidget);
final hoverContentFinderWithTitle = find.text('Hover Content');
expect(hoverContentFinderWithTitle, findsOneWidget);

final containerWithTitle = find
.ancestor(
of: hoverContentFinderWithTitle,
matching: find.byType(Container),
)
.last;
final containerWithTitle = find
.ancestor(
of: hoverContentFinderWithTitle,
matching: find.byType(Container),
)
.last;

final renderBoxWithTitle =
tester.renderObject(containerWithTitle) as RenderBox;
final positionWithTitle = renderBoxWithTitle.localToGlobal(Offset.zero);
final renderBoxWithTitle =
tester.renderObject(containerWithTitle) as RenderBox;
final positionWithTitle = renderBoxWithTitle.localToGlobal(Offset.zero);

// Clamps strictly at y = 274.0 because of dynamic height containing title/divider.
expect(positionWithTitle.dy, equals(274.0));
},
);
// Clamps strictly at y = 274.0 because of dynamic height containing title/divider.
expect(positionWithTitle.dy, equals(274.0));
Comment on lines +143 to +144
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

[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
  1. Strict avoidance of raw values in UI (use named constants) and general code quality (DRY, Meaningful Naming). (link)

});

testWidgetsWithWindowSize(
'HoverCard height clamping without title',
const Size(800, 600),
_windowSize,
(WidgetTester tester) async {
await pumpHoverCardTooltip(tester, alignment: Alignment.bottomCenter);

Expand All @@ -166,39 +171,34 @@ void main() {

testWidgetsWithWindowSize(
'HoverCard translates global coordinates to local coordinates for offset overlays',
const Size(800, 600),
_windowSize,
(WidgetTester tester) async {
final overlayKey = GlobalKey();

await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Padding(
padding: const EdgeInsets.only(left: 50.0, top: 100.0),
child: Provider<HoverCardController>.value(
value: HoverCardController(),
child: Overlay(
key: overlayKey,
initialEntries: [
OverlayEntry(
builder: (context) => Align(
alignment: Alignment.topLeft,
child: HoverCardTooltip.sync(
enabled: () => true,
generateHoverCardData: (event) => HoverCardData(
contents: const SizedBox(
width: 200,
height: 250,
child: Text('Hover Content'),
),
),
child: const Text('Hover Me Offset'),
wrapSimple(
Padding(
padding: const EdgeInsets.only(left: 50.0, top: 100.0),
child: Overlay(
key: overlayKey,
initialEntries: [
OverlayEntry(
builder: (context) => Align(
alignment: Alignment.topLeft,
child: HoverCardTooltip.sync(
enabled: () => true,
generateHoverCardData: (event) => HoverCardData(
contents: const SizedBox(
width: 200,
height: 250,
child: Text('Hover Content'),
),
),
child: const Text('Hover Me Offset'),
),
],
),
),
),
],
),
),
),
Expand All @@ -210,7 +210,7 @@ void main() {

final center = tester.getCenter(find.text('Hover Me Offset'));
await gesture.moveTo(center);
await tester.pump(const Duration(milliseconds: 500));
await tester.pump(HoverCardTooltip.hoverDelay);
await tester.pumpAndSettle();

final hoverContentFinder = find.text('Hover Content');
Expand Down
Loading