From 259afec9e0ce285cfc9fb85ad1b416fffdc6c731 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 13 May 2024 12:55:39 -0600 Subject: [PATCH 01/12] content test: Separate GlobalTime font-size-ratio checks into two tests --- test/widgets/content_test.dart | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 0aaf800e8d..45ed37470f 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -717,8 +717,8 @@ void main() { ..equals(Colors.green); }); - testWidgets('maintains font-size ratio with surrounding text', (tester) async { - Future doCheck(double Function(GlobalTime widget) sizeFromWidget) async { + group('maintains font-size ratio with surrounding text', () { + Future doCheck(WidgetTester tester, double Function(GlobalTime widget) sizeFromWidget) async { await checkFontSizeRatio(tester, targetHtml: '', targetFontSizeFinder: (rootSpan) { @@ -734,21 +734,23 @@ void main() { }); } - // Text is scaled - await doCheck((widget) { - final textSpan = tester.renderObject( - find.descendant(of: find.byWidget(widget), - matching: find.textContaining(renderedTextRegexp) - )).text; - return mergedStyleOfSubstring(textSpan, renderedTextRegexp)!.fontSize!; + testWidgets('text is scaled', (tester) async { + await doCheck(tester, (widget) { + final textSpan = tester.renderObject( + find.descendant(of: find.byWidget(widget), + matching: find.textContaining(renderedTextRegexp) + )).text; + return mergedStyleOfSubstring(textSpan, renderedTextRegexp)!.fontSize!; + }); }); - // Clock icon is scaled - await doCheck((widget) { - final icon = tester.widget( - find.descendant(of: find.byWidget(widget), - matching: find.byIcon(ZulipIcons.clock))); - return icon.size!; + testWidgets('clock icon is scaled', (tester) async { + await doCheck(tester, (widget) { + final icon = tester.widget( + find.descendant(of: find.byWidget(widget), + matching: find.byIcon(ZulipIcons.clock))); + return icon.size!; + }); }); }); }); From 7ae1aebee381c1882879c2f7c2efc8e3b39ed9de Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 13 May 2024 10:59:48 -0600 Subject: [PATCH 02/12] content test [nfc]: Use prepareContentBare where it's a drop-in replacement --- test/widgets/content_test.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 45ed37470f..8777460a74 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -689,8 +689,7 @@ void main() { final renderedTextRegexp = RegExp(r'^(Tue, Jan 30|Wed, Jan 31), 2024, \d+:\d\d [AP]M$'); testWidgets('smoke', (tester) async { - await tester.pumpWidget(MaterialApp(home: BlockContentList(nodes: - parseContent('

$timeSpanHtml

').nodes))); + await prepareContentBare(tester, '

$timeSpanHtml

'); tester.widget(find.textContaining(renderedTextRegexp)); }); From 81772403b7143c7915cb24ae2bb3c1aa61da158e Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 13 May 2024 11:07:33 -0600 Subject: [PATCH 03/12] content test [nfc]: Separate out some steps of `prepareContentBare` --- test/widgets/content_test.dart | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 8777460a74..d6962c89a4 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -93,16 +93,17 @@ void main() { TestZulipBinding.ensureInitialized(); Future prepareContentBare(WidgetTester tester, String html) async { - await tester.pumpWidget(Builder( - builder: (context) { - return MaterialApp( + Widget widget = BlockContentList(nodes: parseContent(html).nodes); + + widget = Scaffold(body: widget); + + await tester.pumpWidget( + Builder(builder: (context) => + MaterialApp( theme: ThemeData(typography: zulipTypography(context)), localizationsDelegates: ZulipLocalizations.localizationsDelegates, supportedLocales: ZulipLocalizations.supportedLocales, - home: Scaffold(body: BlockContentList(nodes: parseContent(html).nodes)), - ); - } - )); + home: widget))); } /// Test that the given content example renders without throwing an exception. From f6b9cffa69fe9accf3c15b071f66ce3e9d8bc474 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 13 May 2024 12:08:46 -0600 Subject: [PATCH 04/12] content test: In prepareContentBare, stop including Scaffold None of our content widgets currently depend on a Scaffold. --- test/widgets/content_test.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index d6962c89a4..d64cd64b5b 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -95,8 +95,6 @@ void main() { Future prepareContentBare(WidgetTester tester, String html) async { Widget widget = BlockContentList(nodes: parseContent(html).nodes); - widget = Scaffold(body: widget); - await tester.pumpWidget( Builder(builder: (context) => MaterialApp( From 306cbec7b95bcb3a68e609c2c90e7d8e3b1cc725 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 13 May 2024 12:58:23 -0600 Subject: [PATCH 05/12] content test: Include GlobalStoreWidget in prepareContentBare --- test/widgets/content_test.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index d64cd64b5b..d9c7b1daf3 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -95,6 +95,9 @@ void main() { Future prepareContentBare(WidgetTester tester, String html) async { Widget widget = BlockContentList(nodes: parseContent(html).nodes); + widget = GlobalStoreWidget(child: widget); + addTearDown(testBinding.reset); + await tester.pumpWidget( Builder(builder: (context) => MaterialApp( @@ -102,6 +105,7 @@ void main() { localizationsDelegates: ZulipLocalizations.localizationsDelegates, supportedLocales: ZulipLocalizations.supportedLocales, home: widget))); + await tester.pump(); // global store } /// Test that the given content example renders without throwing an exception. From c58c8b9fa88a949ed0816eafb2e52a50d52f5a1c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 13 May 2024 12:21:07 -0600 Subject: [PATCH 06/12] content [nfc]: Add wrapWithPerAccountStoreWidget to prepareContentBare We'll use this soon. --- test/widgets/content_test.dart | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index d9c7b1daf3..2a2f1aa255 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -92,9 +92,16 @@ void main() { TestZulipBinding.ensureInitialized(); - Future prepareContentBare(WidgetTester tester, String html) async { + Future prepareContentBare(WidgetTester tester, String html, { + bool wrapWithPerAccountStoreWidget = false, + }) async { Widget widget = BlockContentList(nodes: parseContent(html).nodes); + if (wrapWithPerAccountStoreWidget) { + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + widget = PerAccountStoreWidget(accountId: eg.selfAccount.id, child: widget); + } + widget = GlobalStoreWidget(child: widget); addTearDown(testBinding.reset); @@ -106,6 +113,9 @@ void main() { supportedLocales: ZulipLocalizations.supportedLocales, home: widget))); await tester.pump(); // global store + if (wrapWithPerAccountStoreWidget) { + await tester.pump(); + } } /// Test that the given content example renders without throwing an exception. From a4a7202aa55db573e07ba2bd6040fc1ce0b76c87 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 13 May 2024 12:26:32 -0600 Subject: [PATCH 07/12] content test [nfc]: Add navObservers param to prepareContentBare Callers can use this to check pushed routes. --- test/widgets/content_test.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 2a2f1aa255..14298ea760 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -93,6 +93,7 @@ void main() { TestZulipBinding.ensureInitialized(); Future prepareContentBare(WidgetTester tester, String html, { + List navObservers = const [], bool wrapWithPerAccountStoreWidget = false, }) async { Widget widget = BlockContentList(nodes: parseContent(html).nodes); @@ -111,6 +112,7 @@ void main() { theme: ThemeData(typography: zulipTypography(context)), localizationsDelegates: ZulipLocalizations.localizationsDelegates, supportedLocales: ZulipLocalizations.supportedLocales, + navigatorObservers: navObservers, home: widget))); await tester.pump(); // global store if (wrapWithPerAccountStoreWidget) { From e3b6296ca75f0c0e910421e98fed8c0c7f46a30d Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 13 May 2024 13:16:02 -0600 Subject: [PATCH 08/12] content test [nfc]: Have prepareContentBare prepare test image HTTP client --- test/widgets/content_test.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 14298ea760..18fd6bcc25 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -106,6 +106,8 @@ void main() { widget = GlobalStoreWidget(child: widget); addTearDown(testBinding.reset); + prepareBoringImageHttpClient(); + await tester.pumpWidget( Builder(builder: (context) => MaterialApp( @@ -118,6 +120,8 @@ void main() { if (wrapWithPerAccountStoreWidget) { await tester.pump(); } + + debugNetworkImageHttpClientProvider = null; } /// Test that the given content example renders without throwing an exception. From 08f31a3754e306a329669ab75caa4919f3c18fcc Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 13 May 2024 12:29:21 -0600 Subject: [PATCH 09/12] content test [nfc]: Take `child` instead of `html` --- test/widgets/content_test.dart | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 18fd6bcc25..da5d4fba30 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -92,11 +92,15 @@ void main() { TestZulipBinding.ensureInitialized(); - Future prepareContentBare(WidgetTester tester, String html, { + Widget plainContent(String html) { + return BlockContentList(nodes: parseContent(html).nodes); + } + + Future prepareContentBare(WidgetTester tester, Widget child, { List navObservers = const [], bool wrapWithPerAccountStoreWidget = false, }) async { - Widget widget = BlockContentList(nodes: parseContent(html).nodes); + Widget widget = child; if (wrapWithPerAccountStoreWidget) { await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); @@ -132,7 +136,7 @@ void main() { /// and write an appropriate content-has-rendered check directly. void testContentSmoke(ContentExample example) { testWidgets('smoke: ${example.description}', (tester) async { - await prepareContentBare(tester, example.html); + await prepareContentBare(tester, plainContent(example.html)); assert(example.expectedText != null, 'testContentExample requires expectedText'); tester.widget(find.text(example.expectedText!)); @@ -141,7 +145,7 @@ void main() { group('ThematicBreak', () { testWidgets('smoke ThematicBreak', (tester) async { - await prepareContentBare(tester, ContentExample.thematicBreak.html); + await prepareContentBare(tester, plainContent(ContentExample.thematicBreak.html)); tester.widget(find.byType(ThematicBreak)); }); }); @@ -150,14 +154,14 @@ void main() { testWidgets('plain h6', (tester) async { await prepareContentBare(tester, // "###### six" - '
six
'); + plainContent('
six
')); tester.widget(find.text('six')); }); testWidgets('smoke test for h1, h2, h3, h4, h5', (tester) async { await prepareContentBare(tester, // "# one\n## two\n### three\n#### four\n##### five" - '

one

\n

two

\n

three

\n

four

\n
five
'); + plainContent('

one

\n

two

\n

three

\n

four

\n
five
')); check(find.byType(Heading).evaluate()).length.equals(5); }); }); @@ -454,9 +458,9 @@ void main() { required String targetHtml, required double Function(InlineSpan rootSpan) targetFontSizeFinder, }) async { - await prepareContentBare(tester, + await prepareContentBare(tester, plainContent( '

header-plain $targetHtml

\n' - '

paragraph-plain $targetHtml

'); + '

paragraph-plain $targetHtml

')); final headerRootSpan = tester.renderObject(find.textContaining('header')).text; final headerPlainStyle = mergedStyleOfSubstring(headerRootSpan, 'header-plain '); @@ -708,7 +712,7 @@ void main() { final renderedTextRegexp = RegExp(r'^(Tue, Jan 30|Wed, Jan 31), 2024, \d+:\d\d [AP]M$'); testWidgets('smoke', (tester) async { - await prepareContentBare(tester, '

$timeSpanHtml

'); + await prepareContentBare(tester, plainContent('

$timeSpanHtml

')); tester.widget(find.textContaining(renderedTextRegexp)); }); From 9a258190de22b564f0e40fdcd4f54b4af5fedc62 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 13 May 2024 13:25:21 -0600 Subject: [PATCH 10/12] content test [nfc]: Use prepareContentBare, not ad hoc tester.pumpWidget 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. 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 --- test/widgets/content_test.dart | 159 +++++++++++++-------------------- 1 file changed, 62 insertions(+), 97 deletions(-) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index da5d4fba30..904033c0aa 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -96,6 +96,14 @@ void main() { return BlockContentList(nodes: parseContent(html).nodes); } + Widget messageContent(String html) { + return MessageContent(message: eg.streamMessage(content: html), + content: parseContent(html)); + } + + // TODO(#488) For content that we need to show outside a per-message context + // or a context without a full PerAccountStore, make sure to include tests + // that don't provide such context. Future prepareContentBare(WidgetTester tester, Widget child, { List navObservers = const [], bool wrapWithPerAccountStoreWidget = false, @@ -173,27 +181,16 @@ void main() { group('interactions: spoiler with tappable content (an image) in the header', () { Future>> prepareContent(WidgetTester tester, String html) async { - addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - prepareBoringImageHttpClient(); - final pushedRoutes = >[]; 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; + await prepareContentBare(tester, + // Message is needed for the image's lightbox. + messageContent(html), + navObservers: [testNavObserver], + // We try to resolve the image's URL on the self-account's realm. + wrapWithPerAccountStoreWidget: true); + // `tester.pumpWidget` in prepareContentBare introduces an initial route; // remove it so consumers only have newly pushed routes. assert(pushedRoutes.length == 1); pushedRoutes.removeLast(); @@ -261,18 +258,12 @@ void main() { group('MessageImage, MessageImageList', () { Future 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, + // Message is needed for an image's lightbox. + messageContent(html), + // We try to resolve image URLs on the self-account's realm. + // For URLs on the self-account's realm, we include the auth credential. + wrapWithPerAccountStoreWidget: true); } testWidgets('single image', (tester) async { @@ -353,22 +344,21 @@ void main() { group("MessageInlineVideo", () { Future>> prepareContent(WidgetTester tester, String html) async { - addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - final pushedRoutes = >[]; 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 - + await prepareContentBare(tester, + // Message is needed for a video's lightbox. + messageContent(html), + navObservers: [testNavObserver], + // We try to resolve video URLs on the self-account's realm. + // With #656, we'll show a preview image. We'll try to resolve this + // image's URL on the self-account's realm. If it's on the + // self-account's realm, we'll request it with the auth credential. + // TODO(#656) in above comment, change "we will" to "we do" + wrapWithPerAccountStoreWidget: true); + // `tester.pumpWidget` in prepareContentBare introduces an initial route; + // remove it so consumers only have newly pushed routes. assert(pushedRoutes.length == 1); pushedRoutes.removeLast(); return pushedRoutes; @@ -386,18 +376,11 @@ void main() { group("MessageEmbedVideo", () { Future 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, + // Message is needed for a video's lightbox. + messageContent(html), + // We try to resolve a video preview URL on the self-account's realm. + wrapWithPerAccountStoreWidget: true); } Future checkEmbedVideo(WidgetTester tester, ContentExample example) async { @@ -537,17 +520,9 @@ void main() { // We use this to simulate taps on specific glyphs. Future 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, plainContent(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 { @@ -636,32 +611,29 @@ void main() { }); group('LinkNode on internal links', () { - Future>> 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); + Future>> prepareContent(WidgetTester tester, String html) async { final pushedRoutes = >[]; 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. + + await prepareContentBare(tester, plainContent(html), + navObservers: [testNavObserver], + // We try to resolve relative links on the self-account's realm. + wrapWithPerAccountStoreWidget: true); + + // `tester.pumpWidget` in prepareContentBare introduces an initial route; + // remove it so consumers only have newly pushed routes. assert(pushedRoutes.length == 1); pushedRoutes.removeLast(); + + 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: '

stream

'); + '

stream

'); await tapText(tester, find.text('stream')); check(testBinding.takeLaunchUrlCalls()).isEmpty(); @@ -672,7 +644,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: '

invalid

'); + '

invalid

'); await tapText(tester, find.text('invalid')); final expectedUrl = eg.realmUrl.resolve('/#narrow/stream/1-check/topic'); @@ -717,11 +689,9 @@ 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('

$timeSpanHtml

').nodes), - ))); + await prepareContentBare(tester, + DefaultTextStyle(style: const TextStyle(color: Colors.green), + child: plainContent('

$timeSpanHtml

'))); final icon = tester.widget( find.descendant(of: find.byType(GlobalTime), @@ -779,15 +749,10 @@ void main() { group('MessageImageEmoji', () { Future 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, plainContent(html), + // We try to resolve image-emoji URLs on the self-account's realm. + // For URLs on the self-account's realm, we include the auth credential. + wrapWithPerAccountStoreWidget: true); } testWidgets('smoke: custom emoji', (tester) async { From 40f93afe633f4fe442e6fe00e9ab544f879cbd05 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 15 May 2024 13:16:47 -0700 Subject: [PATCH 11/12] content test [nfc]: s/prepareContent/prepare/ --- test/widgets/content_test.dart | 62 +++++++++++++++++----------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 904033c0aa..8ccd6c0316 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -180,7 +180,7 @@ void main() { testContentSmoke(ContentExample.spoilerRichHeaderAndContent); group('interactions: spoiler with tappable content (an image) in the header', () { - Future>> prepareContent(WidgetTester tester, String html) async { + Future>> prepare(WidgetTester tester, String html) async { final pushedRoutes = >[]; final testNavObserver = TestNavigatorObserver() ..onPushed = (route, prevRoute) => pushedRoutes.add(route); @@ -213,7 +213,7 @@ void main() { const example = ContentExample.spoilerHeaderHasImage; testWidgets('tap image', (tester) async { - final pushedRoutes = await prepareContent(tester, example.html); + final pushedRoutes = await prepare(tester, example.html); await tester.tap(find.byType(RealmContentNetworkImage)); check(pushedRoutes).single.isA() @@ -221,7 +221,7 @@ void main() { }); testWidgets('tap header on expand/collapse icon', (tester) async { - final pushedRoutes = await prepareContent(tester, example.html); + final pushedRoutes = await prepare(tester, example.html); checkIsExpanded(tester, false); await tester.tap(find.byIcon(Icons.expand_more)); @@ -236,7 +236,7 @@ void main() { }); testWidgets('tap header away from expand/collapse icon (and image)', (tester) async { - final pushedRoutes = await prepareContent(tester, example.html); + final pushedRoutes = await prepare(tester, example.html); checkIsExpanded(tester, false); await tester.tapAt( @@ -257,7 +257,7 @@ void main() { testContentSmoke(ContentExample.quotation); group('MessageImage, MessageImageList', () { - Future prepareContent(WidgetTester tester, String html) async { + Future prepare(WidgetTester tester, String html) async { await prepareContentBare(tester, // Message is needed for an image's lightbox. messageContent(html), @@ -268,7 +268,7 @@ void main() { testWidgets('single image', (tester) async { const example = ContentExample.imageSingle; - await prepareContent(tester, example.html); + await prepare(tester, example.html); final expectedImages = (example.expectedNodes[0] as ImageNodeList).images; final images = tester.widgetList( find.byType(RealmContentNetworkImage)); @@ -278,7 +278,7 @@ void main() { testWidgets('image with invalid src URL', (tester) async { const example = ContentExample.imageInvalidUrl; - await prepareContent(tester, example.html); + await prepare(tester, example.html); // The image indeed has an invalid URL. final expectedImages = (example.expectedNodes[0] as ImageNodeList).images; check(() => Uri.parse(expectedImages.single.srcUrl)).throws(); @@ -290,7 +290,7 @@ void main() { testWidgets('multiple images', (tester) async { const example = ContentExample.imageCluster; - await prepareContent(tester, example.html); + await prepare(tester, example.html); final expectedImages = (example.expectedNodes[1] as ImageNodeList).images; final images = tester.widgetList( find.byType(RealmContentNetworkImage)); @@ -300,7 +300,7 @@ void main() { testWidgets('content after image cluster', (tester) async { const example = ContentExample.imageClusterThenContent; - await prepareContent(tester, example.html); + await prepare(tester, example.html); final expectedImages = (example.expectedNodes[1] as ImageNodeList).images; final images = tester.widgetList( find.byType(RealmContentNetworkImage)); @@ -310,7 +310,7 @@ void main() { testWidgets('multiple clusters of images', (tester) async { const example = ContentExample.imageMultipleClusters; - await prepareContent(tester, example.html); + await prepare(tester, example.html); final expectedImages = (example.expectedNodes[1] as ImageNodeList).images + (example.expectedNodes[4] as ImageNodeList).images; final images = tester.widgetList( @@ -321,7 +321,7 @@ void main() { testWidgets('image as immediate child in implicit paragraph', (tester) async { const example = ContentExample.imageInImplicitParagraph; - await prepareContent(tester, example.html); + await prepare(tester, example.html); final expectedImages = ((example.expectedNodes[0] as ListNode) .items[0][0] as ImageNodeList).images; final images = tester.widgetList( @@ -332,7 +332,7 @@ void main() { testWidgets('image cluster in implicit paragraph', (tester) async { const example = ContentExample.imageClusterInImplicitParagraph; - await prepareContent(tester, example.html); + await prepare(tester, example.html); final expectedImages = ((example.expectedNodes[0] as ListNode) .items[0][1] as ImageNodeList).images; final images = tester.widgetList( @@ -343,7 +343,7 @@ void main() { }); group("MessageInlineVideo", () { - Future>> prepareContent(WidgetTester tester, String html) async { + Future>> prepare(WidgetTester tester, String html) async { final pushedRoutes = >[]; final testNavObserver = TestNavigatorObserver() ..onPushed = (route, prevRoute) => pushedRoutes.add(route); @@ -366,7 +366,7 @@ void main() { testWidgets('tapping on preview opens lightbox', (tester) async { const example = ContentExample.videoInline; - final pushedRoutes = await prepareContent(tester, example.html); + final pushedRoutes = await prepare(tester, example.html); await tester.tap(find.byIcon(Icons.play_arrow_rounded)); check(pushedRoutes).single.isA() @@ -375,7 +375,7 @@ void main() { }); group("MessageEmbedVideo", () { - Future prepareContent(WidgetTester tester, String html) async { + Future prepare(WidgetTester tester, String html) async { await prepareContentBare(tester, // Message is needed for a video's lightbox. messageContent(html), @@ -384,7 +384,7 @@ void main() { } Future checkEmbedVideo(WidgetTester tester, ContentExample example) async { - await prepareContent(tester, example.html); + await prepare(tester, example.html); final expectedTitle = (((example.expectedNodes[0] as ParagraphNode) .nodes.single as LinkNode).nodes.single as TextNode).text; @@ -519,14 +519,14 @@ void main() { // https://github.com/flutter/flutter/wiki/Flutter-Test-Fonts // We use this to simulate taps on specific glyphs. - Future prepareContent(WidgetTester tester, String html) async { + Future prepare(WidgetTester tester, String html) async { await prepareContentBare(tester, plainContent(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 { - await prepareContent(tester, + await prepare(tester, '

hello

'); await tapText(tester, find.text('hello')); @@ -540,7 +540,7 @@ void main() { testWidgets('multiple links in paragraph', (tester) async { final fontSize = Paragraph.textStyle.fontSize!; - await prepareContent(tester, + await prepare(tester, '

foo bar baz

'); final base = tester.getTopLeft(find.text('foo bar baz')) .translate(fontSize/2, fontSize/2); // middle of first letter @@ -558,7 +558,7 @@ void main() { }); testWidgets('link nested in other spans', (tester) async { - await prepareContent(tester, + await prepare(tester, '

word

'); await tapText(tester, find.text('word')); check(testBinding.takeLaunchUrlCalls()) @@ -568,7 +568,7 @@ void main() { testWidgets('link containing other spans', (tester) async { final fontSize = Paragraph.textStyle.fontSize!; - await prepareContent(tester, + await prepare(tester, '

two words

'); final base = tester.getTopLeft(find.text('two words')) .translate(fontSize/2, fontSize/2); // middle of first letter @@ -583,7 +583,7 @@ void main() { }); testWidgets('relative links are resolved', (tester) async { - await prepareContent(tester, + await prepare(tester, '

word

'); await tapText(tester, find.text('word')); check(testBinding.takeLaunchUrlCalls()) @@ -591,7 +591,7 @@ void main() { }); testWidgets('link inside HeadingNode', (tester) async { - await prepareContent(tester, + await prepare(tester, '
word
'); await tapText(tester, find.text('word')); check(testBinding.takeLaunchUrlCalls()) @@ -599,7 +599,7 @@ void main() { }); testWidgets('error dialog if invalid link', (tester) async { - await prepareContent(tester, + await prepare(tester, '

word

'); testBinding.launchUrlResult = false; await tapText(tester, find.text('word')); @@ -611,7 +611,7 @@ void main() { }); group('LinkNode on internal links', () { - Future>> prepareContent(WidgetTester tester, String html) async { + Future>> prepare(WidgetTester tester, String html) async { final pushedRoutes = >[]; final testNavObserver = TestNavigatorObserver() ..onPushed = (route, prevRoute) => pushedRoutes.add(route); @@ -632,7 +632,7 @@ void main() { } testWidgets('valid internal links are navigated to within app', (tester) async { - final pushedRoutes = await prepareContent(tester, + final pushedRoutes = await prepare(tester, '

stream

'); await tapText(tester, find.text('stream')); @@ -643,7 +643,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, + final pushedRoutes = await prepare(tester, '

invalid

'); await tapText(tester, find.text('invalid')); @@ -748,7 +748,7 @@ void main() { }); group('MessageImageEmoji', () { - Future prepareContent(WidgetTester tester, String html) async { + Future prepare(WidgetTester tester, String html) async { await prepareContentBare(tester, plainContent(html), // We try to resolve image-emoji URLs on the self-account's realm. // For URLs on the self-account's realm, we include the auth credential. @@ -756,20 +756,20 @@ void main() { } testWidgets('smoke: custom emoji', (tester) async { - await prepareContent(tester, ContentExample.emojiCustom.html); + await prepare(tester, ContentExample.emojiCustom.html); tester.widget(find.byType(MessageImageEmoji)); debugNetworkImageHttpClientProvider = null; }); testWidgets('smoke: custom emoji with invalid URL', (tester) async { - await prepareContent(tester, ContentExample.emojiCustomInvalidUrl.html); + await prepare(tester, ContentExample.emojiCustomInvalidUrl.html); final url = tester.widget(find.byType(MessageImageEmoji)).node.src; check(() => Uri.parse(url)).throws(); debugNetworkImageHttpClientProvider = null; }); testWidgets('smoke: Zulip extra emoji', (tester) async { - await prepareContent(tester, ContentExample.emojiZulipExtra.html); + await prepare(tester, ContentExample.emojiZulipExtra.html); tester.widget(find.byType(MessageImageEmoji)); debugNetworkImageHttpClientProvider = null; }); From 19ed919c90895a28843b0ebdf29af94a7e2c5eb7 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 15 May 2024 13:17:39 -0700 Subject: [PATCH 12/12] content test [nfc]: s/prepareContentBare/prepareContent/ --- test/widgets/content_test.dart | 38 +++++++++++++++++----------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 8ccd6c0316..9d2387c279 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -104,7 +104,7 @@ void main() { // TODO(#488) For content that we need to show outside a per-message context // or a context without a full PerAccountStore, make sure to include tests // that don't provide such context. - Future prepareContentBare(WidgetTester tester, Widget child, { + Future prepareContent(WidgetTester tester, Widget child, { List navObservers = const [], bool wrapWithPerAccountStoreWidget = false, }) async { @@ -140,11 +140,11 @@ void main() { /// /// This requires [ContentExample.expectedText] to be non-null in order to /// check that the content has actually rendered. For examples where there's - /// no suitable value for [ContentExample.expectedText], use [prepareContentBare] + /// no suitable value for [ContentExample.expectedText], use [prepareContent] /// and write an appropriate content-has-rendered check directly. void testContentSmoke(ContentExample example) { testWidgets('smoke: ${example.description}', (tester) async { - await prepareContentBare(tester, plainContent(example.html)); + await prepareContent(tester, plainContent(example.html)); assert(example.expectedText != null, 'testContentExample requires expectedText'); tester.widget(find.text(example.expectedText!)); @@ -153,21 +153,21 @@ void main() { group('ThematicBreak', () { testWidgets('smoke ThematicBreak', (tester) async { - await prepareContentBare(tester, plainContent(ContentExample.thematicBreak.html)); + await prepareContent(tester, plainContent(ContentExample.thematicBreak.html)); tester.widget(find.byType(ThematicBreak)); }); }); group('Heading', () { testWidgets('plain h6', (tester) async { - await prepareContentBare(tester, + await prepareContent(tester, // "###### six" plainContent('
six
')); tester.widget(find.text('six')); }); testWidgets('smoke test for h1, h2, h3, h4, h5', (tester) async { - await prepareContentBare(tester, + await prepareContent(tester, // "# one\n## two\n### three\n#### four\n##### five" plainContent('

one

\n

two

\n

three

\n

four

\n
five
')); check(find.byType(Heading).evaluate()).length.equals(5); @@ -184,13 +184,13 @@ void main() { final pushedRoutes = >[]; final testNavObserver = TestNavigatorObserver() ..onPushed = (route, prevRoute) => pushedRoutes.add(route); - await prepareContentBare(tester, + await prepareContent(tester, // Message is needed for the image's lightbox. messageContent(html), navObservers: [testNavObserver], // We try to resolve the image's URL on the self-account's realm. wrapWithPerAccountStoreWidget: true); - // `tester.pumpWidget` in prepareContentBare introduces an initial route; + // `tester.pumpWidget` in prepareContent introduces an initial route; // remove it so consumers only have newly pushed routes. assert(pushedRoutes.length == 1); pushedRoutes.removeLast(); @@ -258,7 +258,7 @@ void main() { group('MessageImage, MessageImageList', () { Future prepare(WidgetTester tester, String html) async { - await prepareContentBare(tester, + await prepareContent(tester, // Message is needed for an image's lightbox. messageContent(html), // We try to resolve image URLs on the self-account's realm. @@ -347,7 +347,7 @@ void main() { final pushedRoutes = >[]; final testNavObserver = TestNavigatorObserver() ..onPushed = (route, prevRoute) => pushedRoutes.add(route); - await prepareContentBare(tester, + await prepareContent(tester, // Message is needed for a video's lightbox. messageContent(html), navObservers: [testNavObserver], @@ -357,7 +357,7 @@ void main() { // self-account's realm, we'll request it with the auth credential. // TODO(#656) in above comment, change "we will" to "we do" wrapWithPerAccountStoreWidget: true); - // `tester.pumpWidget` in prepareContentBare introduces an initial route; + // `tester.pumpWidget` in prepareContent introduces an initial route; // remove it so consumers only have newly pushed routes. assert(pushedRoutes.length == 1); pushedRoutes.removeLast(); @@ -376,7 +376,7 @@ void main() { group("MessageEmbedVideo", () { Future prepare(WidgetTester tester, String html) async { - await prepareContentBare(tester, + await prepareContent(tester, // Message is needed for a video's lightbox. messageContent(html), // We try to resolve a video preview URL on the self-account's realm. @@ -441,7 +441,7 @@ void main() { required String targetHtml, required double Function(InlineSpan rootSpan) targetFontSizeFinder, }) async { - await prepareContentBare(tester, plainContent( + await prepareContent(tester, plainContent( '

header-plain $targetHtml

\n' '

paragraph-plain $targetHtml

')); @@ -520,7 +520,7 @@ void main() { // We use this to simulate taps on specific glyphs. Future prepare(WidgetTester tester, String html) async { - await prepareContentBare(tester, plainContent(html), + await prepareContent(tester, plainContent(html), // We try to resolve relative links on the self-account's realm. wrapWithPerAccountStoreWidget: true); } @@ -616,12 +616,12 @@ void main() { final testNavObserver = TestNavigatorObserver() ..onPushed = (route, prevRoute) => pushedRoutes.add(route); - await prepareContentBare(tester, plainContent(html), + await prepareContent(tester, plainContent(html), navObservers: [testNavObserver], // We try to resolve relative links on the self-account's realm. wrapWithPerAccountStoreWidget: true); - // `tester.pumpWidget` in prepareContentBare introduces an initial route; + // `tester.pumpWidget` in prepareContent introduces an initial route; // remove it so consumers only have newly pushed routes. assert(pushedRoutes.length == 1); pushedRoutes.removeLast(); @@ -684,12 +684,12 @@ void main() { final renderedTextRegexp = RegExp(r'^(Tue, Jan 30|Wed, Jan 31), 2024, \d+:\d\d [AP]M$'); testWidgets('smoke', (tester) async { - await prepareContentBare(tester, plainContent('

$timeSpanHtml

')); + await prepareContent(tester, plainContent('

$timeSpanHtml

')); tester.widget(find.textContaining(renderedTextRegexp)); }); testWidgets('clock icon and text are the same color', (tester) async { - await prepareContentBare(tester, + await prepareContent(tester, DefaultTextStyle(style: const TextStyle(color: Colors.green), child: plainContent('

$timeSpanHtml

'))); @@ -749,7 +749,7 @@ void main() { group('MessageImageEmoji', () { Future prepare(WidgetTester tester, String html) async { - await prepareContentBare(tester, plainContent(html), + await prepareContent(tester, plainContent(html), // We try to resolve image-emoji URLs on the self-account's realm. // For URLs on the self-account's realm, we include the auth credential. wrapWithPerAccountStoreWidget: true);