Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@jason-simmons
Copy link
Member

  • Update the tests for current versions of third-party libraries
  • Fix tests that require Android SDK versions 29 and above
    (These tests were being skipped by the old Robolectric configuration)

@jason-simmons jason-simmons requested a review from blasten August 13, 2021 21:45
@google-cla google-cla bot added the cla: yes label Aug 13, 2021
@jason-simmons jason-simmons force-pushed the gradle_robolectric_2 branch 2 times, most recently from 0ad7786 to fe10691 Compare August 13, 2021 23:27

// Verify.
verify(flutterRenderer, times(2)).setViewportMetrics(viewportMetricsCaptor.capture());
assertEquals(0, viewportMetricsCaptor.getValue().viewPaddingTop);
Copy link

Choose a reason for hiding this comment

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

interesting. was this a bug in the test?

Copy link
Member Author

Choose a reason for hiding this comment

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

These tests have not been running because the current Robolectric configuration is skipping anything with an API level above 28. The behavior of the code diverged from the expectations of the test, and AFAICT the code is correct.

// This test uses the pre-API 30 Algorithm for window insets.
@Test
@TargetApi(29)
@TargetApi(28)
Copy link

Choose a reason for hiding this comment

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

was there an error caused by target api = 29?

Copy link
Member Author

Choose a reason for hiding this comment

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

The code behaves differently for API level 29 versus 28, and the test seems to be based on the API 28 implementation.

":flutter_shell_java",
]
# TODO(jsimmons): remove this placeholder when it is no longer used by the LUCI recipes
group("robolectric_tests") {
Copy link

Choose a reason for hiding this comment

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

This rule is useful to makerobolectric_tests depend on android_jar. Do you know if gn has something equivalent to bazel test :<target> or bazel run :<target>?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know of a specialized command like that. But I suppose you could create a GN target with an action that runs run_tests.py --type=java and a dependency on the Android engine JAR.

Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

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

Thanks for the refactor!

It's pretty great to have an idiomatic way of defining test dependencies.

@jason-simmons jason-simmons force-pushed the gradle_robolectric_2 branch 6 times, most recently from 0383ab9 to 3fd16ea Compare August 19, 2021 01:00
* Update the tests for current versions of third-party libraries
* Fix tests that require Android SDK versions 29 and above
  (These tests were being skipped by the old Robolectric configuration)
@jason-simmons jason-simmons added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Aug 25, 2021
@fluttergithubbot fluttergithubbot merged commit 63f83fc into flutter:master Aug 25, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 25, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 25, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 25, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 25, 2021
zanderso pushed a commit to flutter/flutter that referenced this pull request Aug 25, 2021
* f575a5d Roll Dart SDK from abecc4bc4521 to 72f462ed2d34 (6 revisions) (flutter/engine#28279)

* 25262ad Roll Skia from 0f36d11c1dcf to 6f90c702e6c8 (4 revisions) (flutter/engine#28280)

* 6c8ef61 Correct the return value of the method RunInIsolateScope (flutter/engine#28161)

* 2126214 Roll Skia from 6f90c702e6c8 to 877858a19502 (2 revisions) (flutter/engine#28282)

* 62934d0 Eliminate unnecessary null check in fixture (flutter/engine#28281)

* e218f5f Roll Skia from 877858a19502 to e43714f490ec (6 revisions) (flutter/engine#28283)

* b17c2a6 Roll Dart SDK from 72f462ed2d34 to 35c81c55d91f (1 revision) (flutter/engine#28284)

* bba6439 Roll Skia from e43714f490ec to 7b3edfa65923 (1 revision) (flutter/engine#28285)

* 902b16d Roll buildroot to 14b15b2 (flutter/engine#28286)

* 687affe hasStrings on Windows (flutter/engine#27749)

* 33342cb Roll Clang Mac from T2LfgbJBs... to XvHmkqWWH... (flutter/engine#28288)

* 5508373 Roll Clang Linux from cIFYQ49sp... to dO8igrHyL... (flutter/engine#28289)

* c2e289e Roll Skia from 7b3edfa65923 to 80fc31bc61a9 (2 revisions) (flutter/engine#28290)

* 1867ca4 Roll Dart SDK from 35c81c55d91f to 4c4bf41da092 (1 revision) (flutter/engine#28291)

* d75f033 Roll Skia from 80fc31bc61a9 to 9155b338bb9c (1 revision) (flutter/engine#28292)

* 9a2383c Roll Dart SDK from 4c4bf41da092 to 0507607e1380 (1 revision) (flutter/engine#28294)

* 16a94e6 Roll Skia from 9155b338bb9c to 02c7dac0be64 (1 revision) (flutter/engine#28295)

* 64891d0 Roll Skia from 02c7dac0be64 to 0305e86ee48c (1 revision) (flutter/engine#28296)

* 58b1b60 Roll Dart SDK from 0507607e1380 to 2a693f47e303 (1 revision) (flutter/engine#28297)

* 9c78284 Roll Skia from 0305e86ee48c to 1e16937f4734 (2 revisions) (flutter/engine#28300)

* 63f83fc Run the Android Robolectric tests using Gradle (flutter/engine#28084)

* 93f6188 Roll Dart SDK from 2a693f47e303 to 6fecad447467 (1 revision) (flutter/engine#28301)

* d6677ba Roll Skia from 1e16937f4734 to d34406abf959 (1 revision) (flutter/engine#28303)

* 6243b46 Roll Skia from d34406abf959 to b61014d31064 (5 revisions) (flutter/engine#28305)

* eaa9f8b Revert "Roll Dart SDK from 2a693f47e303 to 6fecad447467 (1 revision) (#28301)" (flutter/engine#28309)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes platform-android waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants