Skip to content

Conversation

@moffatman
Copy link
Contributor

WidgetController.startGesture starts with a down event. It's updated here to start with a panZoomStart event if kind is set to PointerDeviceKind.trackpad. The moveBy and up method have also been updated so that the TestGesture can be used in the same way for trackpad as other device kinds, but sending appropriate gesture events for a trackpad. Added some more descriptive assertions when trying to send inappropriate events on a trackpad pointer.

Fixes #113773

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on [Discord].

@flutter-dashboard flutter-dashboard bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Oct 30, 2022
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

This is a great improvement, thanks! Some style/formatting nits, otherwise LGTM

@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #114312 at sha 3732770

@flutter-dashboard flutter-dashboard bot added the will affect goldens Changes to golden files label Nov 1, 2022
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM, will look into google failures

@dnfield
Copy link
Contributor

dnfield commented Nov 1, 2022

Having trouble telling, but it looks like the internal roll pulled in unrelated tool changes and is failing on that.

@dnfield
Copy link
Contributor

dnfield commented Nov 1, 2022

There's a real failure internally. The test is failing on these lines:

        final input = await tester.createGesture(kind: testEntry.key);
        await input.moveTo(const Offset(50, 50));

Asserts

'package:flutter_test/src/test_pointer.dart': Failed assertion:
line 534 pos 12: '_pointer.kind != PointerDeviceKind.trackpad':
failed: ══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞═════════════════
The following assertion was thrown running a test:
'package:flutter_test/src/test_pointer.dart': Failed assertion:
line 534 pos 12: '_pointer.kind != PointerDeviceKind.trackpad':
is not true.

When the exception was thrown, this was the stack:
#2      TestGesture.moveTo (package:flutter_test/src/test_pointer.dart:534:12)
#3      main.<anonymous closure> (redacted/path/input_type_detector_test.dart:73:21)
<asynchronous suspension>
#4      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:171:15)
<asynchronous suspension>
#5      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:858:5)

@dnfield
Copy link
Contributor

dnfield commented Nov 1, 2022

and testEntry.key is iterating through this map:

  final pointerDeviceToInputType = {
    PointerDeviceKind.invertedStylus: InputType.touch,
    PointerDeviceKind.mouse: InputType.keyboardAndMouse,
    PointerDeviceKind.stylus: InputType.touch,
    PointerDeviceKind.touch: InputType.touch,
    PointerDeviceKind.trackpad: InputType.keyboardAndMouse,
    PointerDeviceKind.unknown: InputType.touch,
  };

@dnfield
Copy link
Contributor

dnfield commented Nov 1, 2022

Ahh I see, this test just needs to be updated to skip trackPad now.

@moffatman
Copy link
Contributor Author

I couldn't implement moveTo since a gesture only makes sense with relative movements, not absolute ones. Strange it's only failing on that now, it should have been hitting the later assertion anyways.

@dnfield
Copy link
Contributor

dnfield commented Nov 1, 2022

I've updated the internal test and AFAICT this will now pass.

@dnfield dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 1, 2022
@moffatman
Copy link
Contributor Author

@dnfield What do you recommend to do now about flutter-gold ?

@auto-submit
Copy link
Contributor

auto-submit bot commented Nov 2, 2022

auto label is removed for flutter/flutter, pr: 114312, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 2, 2022
@dnfield
Copy link
Contributor

dnfield commented Nov 3, 2022

Flutter gold seems to be in an error state fo rthis PR and I'm not sure why. @Piinks?

@moffatman
Copy link
Contributor Author

I had the same issue in #97928 but that one was 6 months+ old so seemed reasonable. Maybe I should recreate this one as well?

@Piinks
Copy link
Contributor

Piinks commented Nov 3, 2022

Filed for follow up: https://bugs.chromium.org/p/skia/issues/detail?id=13882

https://flutter-gold.skia.org/json/v1/changelist_summary/github/114312 shows that there are three image changes, but the dashboard does not show anything. If you run the tests locally are there image failures? Local execution will output the affected images.

Also relevant: https://bugs.chromium.org/p/skia/issues/detail?id=13847 it was de-duped with another issue I filed for it. We've been seeing this error pop up more frequently, but functionality has been unaffected - but that may have changed.. I will follow up with the Gold team tomorrow

@dnfield
Copy link
Contributor

dnfield commented Nov 3, 2022

@dnfield What do you recommend to do now about flutter-gold ?

If it's not a problem, would you mind just closing this PR and opening a new one? Sorry about this.

@moffatman
Copy link
Contributor Author

Recreated at #114631

@moffatman moffatman closed this Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. will affect goldens Changes to golden files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Calling WidgetTester.startGesture with a trackpad kind throws

3 participants