Skip to content

Commit

Permalink
content test [nfc]: Deduplicate ad-hoc content-preparing code
Browse files Browse the repository at this point in the history 
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 zulip#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: zulip#95
Related: zulip#488
  • Loading branch information
@chrisbobbe
chrisbobbe committed May 13, 2024
1 parent 431aa4c commit e151e61
Showing 1 changed file with 49 additions and 116 deletions.
165 changes: 49 additions & 116 deletions test/widgets/content_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,18 @@ void main() {

TestZulipBinding.ensureInitialized();

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

group('interactions: spoiler with tappable content (an image) in the header', () {
Future<List<Route<dynamic>>> prepareContent(WidgetTester tester, String html) async {
addTearDown(testBinding.reset);
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
prepareBoringImageHttpClient();

final pushedRoutes = <Route<dynamic>>[];
final testNavObserver = TestNavigatorObserver()
..onPushed = (route, prevRoute) => pushedRoutes.add(route);

await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
localizationsDelegates: ZulipLocalizations.localizationsDelegates,
supportedLocales: ZulipLocalizations.supportedLocales,
navigatorObservers: [testNavObserver],
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
child: MessageContent(
message: eg.streamMessage(content: html),
content: parseContent(html))))));
await tester.pump(); // global store
await tester.pump(); // per-account store
debugNetworkImageHttpClientProvider = null;

// `tester.pumpWidget` introduces an initial route;
// remove it so consumers only have newly pushed routes.
assert(pushedRoutes.length == 1);
pushedRoutes.removeLast();
return pushedRoutes;
return prepareContentBare(tester, html,
// We try to resolve the image's URL on the self-account's realm.
wrapWithPerAccountStoreWidget: true,
// Message is needed for the image's lightbox.
wrapWithMessageContent: true);
}

void checkIsExpanded(WidgetTester tester,
Expand Down Expand Up @@ -284,18 +276,11 @@ void main() {

group('MessageImage, MessageImageList', () {
Future<void> prepareContent(WidgetTester tester, String html) async {
addTearDown(testBinding.reset);
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
prepareBoringImageHttpClient();

await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
child: MessageContent(
message: eg.streamMessage(content: html),
content: parseContent(html))))));
await tester.pump(); // global store
await tester.pump(); // per-account store
debugNetworkImageHttpClientProvider = null;
await prepareContentBare(tester, html,
// We try to resolve image URLs on the self-account's realm.
wrapWithPerAccountStoreWidget: true,
// Message is needed for an image's lightbox.
wrapWithMessageContent: true);
}

testWidgets('single image', (tester) async {
Expand Down Expand Up @@ -376,25 +361,11 @@ void main() {

group("MessageInlineVideo", () {
Future<List<Route<dynamic>>> prepareContent(WidgetTester tester, String html) async {
addTearDown(testBinding.reset);
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());

final pushedRoutes = <Route<dynamic>>[];
final testNavObserver = TestNavigatorObserver()
..onPushed = (route, prevRoute) => pushedRoutes.add(route);

await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
navigatorObservers: [testNavObserver],
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
child: MessageContent(
message: eg.streamMessage(content: html),
content: parseContent(html))))));
await tester.pump(); // global store
await tester.pump(); // per-account store

assert(pushedRoutes.length == 1);
pushedRoutes.removeLast();
return pushedRoutes;
return prepareContentBare(tester, html,
// We try to resolve video URLs on the self-account's realm.
wrapWithPerAccountStoreWidget: true,
// Message is needed for a video's lightbox.
wrapWithMessageContent: true);
}

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

group("MessageEmbedVideo", () {
Future<void> prepareContent(WidgetTester tester, String html) async {
addTearDown(testBinding.reset);
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
prepareBoringImageHttpClient();

await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
child: MessageContent(
message: eg.streamMessage(content: html),
content: parseContent(html))))));
await tester.pump(); // global store
await tester.pump(); // per-account store
debugNetworkImageHttpClientProvider = null;
Future<List<Route<dynamic>>> prepareContent(WidgetTester tester, String html) async {
return prepareContentBare(tester, html,
// We try to resolve a video preview URL on the self-account's realm.
wrapWithPerAccountStoreWidget: true,
// Message is needed for a video's lightbox.
wrapWithMessageContent: true);
}

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

Future<void> prepareContent(WidgetTester tester, String html) async {
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
addTearDown(testBinding.reset);

await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
localizationsDelegates: ZulipLocalizations.localizationsDelegates,
supportedLocales: ZulipLocalizations.supportedLocales,
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
child: BlockContentList(
nodes: parseContent(html).nodes)))));
await tester.pump();
await tester.pump();
await prepareContentBare(tester, html,
// We try to resolve relative links on the self-account's realm.
wrapWithPerAccountStoreWidget: true);
}

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

group('LinkNode on internal links', () {
Future<List<Route<dynamic>>> prepareContent(WidgetTester tester, {
required String html,
}) async {
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot(
streams: [eg.stream(streamId: 1, name: 'check')],
));
addTearDown(testBinding.reset);
final pushedRoutes = <Route<dynamic>>[];
final testNavObserver = TestNavigatorObserver()
..onPushed = (route, prevRoute) => pushedRoutes.add(route);
await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
navigatorObservers: [testNavObserver],
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
child: BlockContentList(nodes: parseContent(html).nodes)))));
await tester.pump(); // global store
await tester.pump(); // per-account store
// `tester.pumpWidget` introduces an initial route, remove so
// consumers only have newly pushed routes.
assert(pushedRoutes.length == 1);
pushedRoutes.removeLast();
Future<List<Route<dynamic>>> prepareContent(WidgetTester tester, String html) async {
final pushedRoutes = await prepareContentBare(tester, html,
// We try to resolve relative links on the self-account's realm.
wrapWithPerAccountStoreWidget: true);
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
store.addStream(eg.stream(name: 'stream'));
return pushedRoutes;
}

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

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

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

testWidgets('clock icon and text are the same color', (tester) async {
await tester.pumpWidget(MaterialApp(home: DefaultTextStyle(
style: const TextStyle(color: Colors.green),
child: BlockContentList(nodes:
parseContent('<p>$timeSpanHtml</p>').nodes),
)));
await prepareContentBare(tester, '<p>$timeSpanHtml</p>',
defaultTextStyleOverride: const TextStyle(color: Colors.green));

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

group('MessageImageEmoji', () {
Future<void> prepareContent(WidgetTester tester, String html) async {
addTearDown(testBinding.reset);
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
prepareBoringImageHttpClient();

await tester.pumpWidget(GlobalStoreWidget(child: MaterialApp(
home: PerAccountStoreWidget(accountId: eg.selfAccount.id,
child: BlockContentList(nodes: parseContent(html).nodes)))));
await tester.pump(); // global store
await tester.pump(); // per-account store
await prepareContentBare(tester, html,
// We try to resolve image-emoji URLs on the self-account's realm.
wrapWithPerAccountStoreWidget: true);
}

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

0 comments on commit e151e61

Please sign in to comment.