Skip to content

Commit 7050c3c

Browse files
committed
content tests [nfc]: Deduplicate ad-hoc content-preparing code
It should now be easy to drop in natural dependencies of our content widgets, such as a ThemeExtension for custom light- and dark-theme colors, for all the tests of our Zulip content widgets. To do that, we can just add unconditional code to this function. It should also help us keep being explicit about some potentially problematic dependencies, which will be handy when we work on #488 ("content: Support Zulip content outside messages (even outside per-account contexts)"). After this change, some ad-hoc `tester.pumpWidget` calls do remain in this file; see tests of RealmContentNetworkImage and AvatarImage. But those aren't "Zulip content widgets" in the same sense as most of the widgets in lib/widgets/content.dart, which are used exclusively to render parsed Zulip Markdown. (There isn't even any Zulip Markdown that would produce an AvatarImage.) So, leave them be. Perhaps these widgets belong in some other file. Related: #95 Related: #488
1 parent 431aa4c commit 7050c3c

File tree

1 file changed

+49
-116
lines changed

1 file changed

+49
-116
lines changed

test/widgets/content_test.dart

Lines changed: 49 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,18 @@ void main() {
9292

9393
TestZulipBinding.ensureInitialized();
9494

95+
/// Parses the given [html] and pumps the corresponding content widgets.
96+
///
97+
/// Use the minimum "extra" dependencies necessary (like
98+
/// [wrapWithPerAccountStoreWidget]), and for each one used, explain why
99+
/// in a code comment. See existing callers for examples.
100+
///
101+
/// When testing content widgets, prefer this function (adapting it as needed)
102+
/// instead of writing ad-hoc content-preparing code. It helps us track our
103+
/// content widgets' dependencies.
104+
// TODO(#488) For content that we need to show outside a per-message or
105+
// per-account context, make sure to include tests that don't provide
106+
// such context.
95107
Future<List<Route<dynamic>>> prepareContentBare(WidgetTester tester, String html, {
96108
bool wrapWithScaffold = false,
97109
bool wrapWithPerAccountStoreWidget = false,
@@ -196,31 +208,11 @@ void main() {
196208

197209
group('interactions: spoiler with tappable content (an image) in the header', () {
198210
Future<List<Route<dynamic>>> prepareContent(WidgetTester tester, String html) async {
199-
addTearDown(testBinding.reset);
200-
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
201-
prepareBoringImageHttpClient();
202-
203-
final pushedRoutes = <Route<dynamic>>[];
204-
final testNavObserver = TestNavigatorObserver()
205-
..onPushed = (route, prevRoute) => pushedRoutes.add(route);
206-
207-
await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
208-
localizationsDelegates: ZulipLocalizations.localizationsDelegates,
209-
supportedLocales: ZulipLocalizations.supportedLocales,
210-
navigatorObservers: [testNavObserver],
211-
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
212-
child: MessageContent(
213-
message: eg.streamMessage(content: html),
214-
content: parseContent(html))))));
215-
await tester.pump(); // global store
216-
await tester.pump(); // per-account store
217-
debugNetworkImageHttpClientProvider = null;
218-
219-
// `tester.pumpWidget` introduces an initial route;
220-
// remove it so consumers only have newly pushed routes.
221-
assert(pushedRoutes.length == 1);
222-
pushedRoutes.removeLast();
223-
return pushedRoutes;
211+
return prepareContentBare(tester, html,
212+
// We try to resolve the image's URL on the self-account's realm.
213+
wrapWithPerAccountStoreWidget: true,
214+
// Message is needed for the image's lightbox.
215+
wrapWithMessageContent: true);
224216
}
225217

226218
void checkIsExpanded(WidgetTester tester,
@@ -284,18 +276,11 @@ void main() {
284276

285277
group('MessageImage, MessageImageList', () {
286278
Future<void> prepareContent(WidgetTester tester, String html) async {
287-
addTearDown(testBinding.reset);
288-
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
289-
prepareBoringImageHttpClient();
290-
291-
await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
292-
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
293-
child: MessageContent(
294-
message: eg.streamMessage(content: html),
295-
content: parseContent(html))))));
296-
await tester.pump(); // global store
297-
await tester.pump(); // per-account store
298-
debugNetworkImageHttpClientProvider = null;
279+
await prepareContentBare(tester, html,
280+
// We try to resolve image URLs on the self-account's realm.
281+
wrapWithPerAccountStoreWidget: true,
282+
// Message is needed for an image's lightbox.
283+
wrapWithMessageContent: true);
299284
}
300285

301286
testWidgets('single image', (tester) async {
@@ -376,25 +361,11 @@ void main() {
376361

377362
group("MessageInlineVideo", () {
378363
Future<List<Route<dynamic>>> prepareContent(WidgetTester tester, String html) async {
379-
addTearDown(testBinding.reset);
380-
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
381-
382-
final pushedRoutes = <Route<dynamic>>[];
383-
final testNavObserver = TestNavigatorObserver()
384-
..onPushed = (route, prevRoute) => pushedRoutes.add(route);
385-
386-
await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
387-
navigatorObservers: [testNavObserver],
388-
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
389-
child: MessageContent(
390-
message: eg.streamMessage(content: html),
391-
content: parseContent(html))))));
392-
await tester.pump(); // global store
393-
await tester.pump(); // per-account store
394-
395-
assert(pushedRoutes.length == 1);
396-
pushedRoutes.removeLast();
397-
return pushedRoutes;
364+
return prepareContentBare(tester, html,
365+
// We try to resolve video URLs on the self-account's realm.
366+
wrapWithPerAccountStoreWidget: true,
367+
// Message is needed for a video's lightbox.
368+
wrapWithMessageContent: true);
398369
}
399370

400371
testWidgets('tapping on preview opens lightbox', (tester) async {
@@ -408,19 +379,12 @@ void main() {
408379
});
409380

410381
group("MessageEmbedVideo", () {
411-
Future<void> prepareContent(WidgetTester tester, String html) async {
412-
addTearDown(testBinding.reset);
413-
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
414-
prepareBoringImageHttpClient();
415-
416-
await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
417-
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
418-
child: MessageContent(
419-
message: eg.streamMessage(content: html),
420-
content: parseContent(html))))));
421-
await tester.pump(); // global store
422-
await tester.pump(); // per-account store
423-
debugNetworkImageHttpClientProvider = null;
382+
Future<List<Route<dynamic>>> prepareContent(WidgetTester tester, String html) async {
383+
return prepareContentBare(tester, html,
384+
// We try to resolve a video preview URL on the self-account's realm.
385+
wrapWithPerAccountStoreWidget: true,
386+
// Message is needed for a video's lightbox.
387+
wrapWithMessageContent: true);
424388
}
425389

426390
Future<void> checkEmbedVideo(WidgetTester tester, ContentExample example) async {
@@ -560,17 +524,9 @@ void main() {
560524
// We use this to simulate taps on specific glyphs.
561525

562526
Future<void> prepareContent(WidgetTester tester, String html) async {
563-
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
564-
addTearDown(testBinding.reset);
565-
566-
await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
567-
localizationsDelegates: ZulipLocalizations.localizationsDelegates,
568-
supportedLocales: ZulipLocalizations.supportedLocales,
569-
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
570-
child: BlockContentList(
571-
nodes: parseContent(html).nodes)))));
572-
await tester.pump();
573-
await tester.pump();
527+
await prepareContentBare(tester, html,
528+
// We try to resolve relative links on the self-account's realm.
529+
wrapWithPerAccountStoreWidget: true);
574530
}
575531

576532
testWidgets('can tap a link to open URL', (tester) async {
@@ -659,32 +615,18 @@ void main() {
659615
});
660616

661617
group('LinkNode on internal links', () {
662-
Future<List<Route<dynamic>>> prepareContent(WidgetTester tester, {
663-
required String html,
664-
}) async {
665-
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot(
666-
streams: [eg.stream(streamId: 1, name: 'check')],
667-
));
668-
addTearDown(testBinding.reset);
669-
final pushedRoutes = <Route<dynamic>>[];
670-
final testNavObserver = TestNavigatorObserver()
671-
..onPushed = (route, prevRoute) => pushedRoutes.add(route);
672-
await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
673-
navigatorObservers: [testNavObserver],
674-
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
675-
child: BlockContentList(nodes: parseContent(html).nodes)))));
676-
await tester.pump(); // global store
677-
await tester.pump(); // per-account store
678-
// `tester.pumpWidget` introduces an initial route, remove so
679-
// consumers only have newly pushed routes.
680-
assert(pushedRoutes.length == 1);
681-
pushedRoutes.removeLast();
618+
Future<List<Route<dynamic>>> prepareContent(WidgetTester tester, String html) async {
619+
final pushedRoutes = await prepareContentBare(tester, html,
620+
// We try to resolve relative links on the self-account's realm.
621+
wrapWithPerAccountStoreWidget: true);
622+
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
623+
store.addStream(eg.stream(name: 'stream'));
682624
return pushedRoutes;
683625
}
684626

685627
testWidgets('valid internal links are navigated to within app', (tester) async {
686628
final pushedRoutes = await prepareContent(tester,
687-
html: '<p><a href="/#narrow/stream/1-check">stream</a></p>');
629+
'<p><a href="/#narrow/stream/1-check">stream</a></p>');
688630

689631
await tapText(tester, find.text('stream'));
690632
check(testBinding.takeLaunchUrlCalls()).isEmpty();
@@ -695,7 +637,7 @@ void main() {
695637
testWidgets('invalid internal links are opened in browser', (tester) async {
696638
// Link is invalid due to `topic` operator missing an operand.
697639
final pushedRoutes = await prepareContent(tester,
698-
html: '<p><a href="/#narrow/stream/1-check/topic">invalid</a></p>');
640+
'<p><a href="/#narrow/stream/1-check/topic">invalid</a></p>');
699641

700642
await tapText(tester, find.text('invalid'));
701643
final expectedUrl = eg.realmUrl.resolve('/#narrow/stream/1-check/topic');
@@ -740,11 +682,8 @@ void main() {
740682
});
741683

742684
testWidgets('clock icon and text are the same color', (tester) async {
743-
await tester.pumpWidget(MaterialApp(home: DefaultTextStyle(
744-
style: const TextStyle(color: Colors.green),
745-
child: BlockContentList(nodes:
746-
parseContent('<p>$timeSpanHtml</p>').nodes),
747-
)));
685+
await prepareContentBare(tester, '<p>$timeSpanHtml</p>',
686+
defaultTextStyleOverride: const TextStyle(color: Colors.green));
748687

749688
final icon = tester.widget<Icon>(
750689
find.descendant(of: find.byType(GlobalTime),
@@ -802,15 +741,9 @@ void main() {
802741

803742
group('MessageImageEmoji', () {
804743
Future<void> prepareContent(WidgetTester tester, String html) async {
805-
addTearDown(testBinding.reset);
806-
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
807-
prepareBoringImageHttpClient();
808-
809-
await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
810-
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
811-
child: BlockContentList(nodes: parseContent(html).nodes)))));
812-
await tester.pump(); // global store
813-
await tester.pump(); // per-account store
744+
await prepareContentBare(tester, html,
745+
// We try to resolve image-emoji URLs on the self-account's realm.
746+
wrapWithPerAccountStoreWidget: true);
814747
}
815748

816749
testWidgets('smoke: custom emoji', (tester) async {

0 commit comments

Comments
 (0)