Skip to content

Flutter update breaks link interaction tests #475

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.

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

Closed
gnprice opened this issue Jan 4, 2024 · 2 comments · Fixed by #476
Closed

Flutter update breaks link interaction tests #475

gnprice opened this issue Jan 4, 2024 · 2 comments · Fixed by #476
Assignees
Labels

Comments

@gnprice
Copy link
Member

gnprice commented Jan 4, 2024

After a Flutter update, some tests are failing in main. This is visible also in the recent PRs #472 and #474, as here:
https://github.com/zulip/zulip-flutter/actions/runs/7391944552/job/20109637276?pr=472

The failure looks like this:

$ flutter test test/widgets/content_test.dart --name 'can tap a link'
00:02 +0: LinkNode interactions can tap a link to open URL (variant: TargetPlatform.android)
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: a List<({LaunchMode mode, Uri url})> that:
  has single element
Actual: []
Which: has no elements

When the exception was thrown, this was the stack:
#0      check.<anonymous closure> (package:checks/src/checks.dart:72:9)
#1      _TestContext.nest (package:checks/src/checks.dart:760:12)
#2      IterableChecks.single (package:checks/src/extensions/iterable.dart:33:36)
#3      main.<anonymous closure>.<anonymous closure> (file:///home/greg/z/flutterz/test/widgets/content_test.dart:90:10)
<asynchronous suspension>
#4      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:168:15)
<asynchronous suspension>
#5      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1017:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

The test description was:
  can tap a link to open URL (variant: TargetPlatform.android)
════════════════════════════════════════════════════════════════════════════════════════════════════
00:02 +0 -1: LinkNode interactions can tap a link to open URL (variant: TargetPlatform.android) [E]
  Test failed. See exception logs above.
  The test description was: can tap a link to open URL (variant: TargetPlatform.android)
  

To run this test again: /home/greg/n/flutter/flutter/bin/cache/dart-sdk/bin/dart test /home/greg/z/flutterz/test/widgets/content_test.dart -p vm --plain-name 'LinkNode interactions can tap a link to open URL (variant: TargetPlatform.android)'

The test renders some content that has a link in it, then taps the link and checks that the tap made it to the binding for launchUrl. The check fails — apparently the tap didn't make it to the binding.

On the other hand, following a link in the actual app seems to work fine. So this may only affect the tests.

More investigation needed.

@gnprice gnprice added the a-tests label Jan 4, 2024
@gnprice gnprice added this to the Beta 2 milestone Jan 4, 2024
@gnprice gnprice self-assigned this Jan 4, 2024
@gnprice
Copy link
Member Author

gnprice commented Jan 4, 2024

OK, here's the upstream change that caused this breakage:

So chalk up another one for #239, then.

The change sounds like a good one. Still investigating to determine why it breaks the tests and what the right fix is.

@gnprice
Copy link
Member Author

gnprice commented Jan 4, 2024

OK, the core of the issue is that we say things like this:

    testWidgets('can tap a link to open URL', (tester) async {
      await prepareContent(tester,
        '<p><a href="https://example/">hello</a></p>');
      await tester.tapAt(find.text('hello'));

and tapAt means tapping at the center of the widget. So we're tapping at the center of a Text widget; but the widget extends the full width, and the actual glyphs laid out as the paragraph inside it only occupy the start of that, without reaching the center.

There's a few possible ways we might fix that: possibly the Text widget we build for a ParagraphNode of Zulip content should only go as wide as its internal layout, or possibly the tests should just change where they tap.

gnprice added a commit to gnprice/zulip-flutter that referenced this issue Jan 4, 2024
By saying `tester.tapAt(find.text('hello'))`, we had been aiming
at the center of the Text widgets, and expecting that to hit the
recognizer we've put on the span, even though the widget is much
wider than the span and the latter doesn't reach the former's center.
Effectively we were relying on the presence of issue zulip#214.

But with an upstream change yesterday:
  flutter/flutter#140621
such a tap no longer hits the span.  That broke these tests.

To fix them, aim the taps near the start of the widget instead.

Fixes: zulip#475
chrisbobbe pushed a commit that referenced this issue Jan 4, 2024
By saying `tester.tapAt(find.text('hello'))`, we had been aiming
at the center of the Text widgets, and expecting that to hit the
recognizer we've put on the span, even though the widget is much
wider than the span and the latter doesn't reach the former's center.
Effectively we were relying on the presence of issue #214.

But with an upstream change yesterday:
  flutter/flutter#140621
such a tap no longer hits the span.  That broke these tests.

To fix them, aim the taps near the start of the widget instead.

Fixes: #475
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant