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

Bump Robolectric to 4.8.1 #33024

Merged
merged 2 commits into from
Jun 23, 2022
Merged

Bump Robolectric to 4.8.1 #33024

merged 2 commits into from
Jun 23, 2022

Conversation

utzcoz
Copy link
Contributor

@utzcoz utzcoz commented Apr 30, 2022

  1. Bump sdk to 32.
  2. Remove icu4j from testImplementation because Robolectric has a higher icu4j dependency.
  3. Fix PlatformViewsControllerTest#disposeNullAndroidView with Robolectric 4.8.x.

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 and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

@utzcoz utzcoz force-pushed the bump-Robolectric branch from 7ab298b to eb352c0 Compare April 30, 2022 07:38
@utzcoz utzcoz force-pushed the bump-Robolectric branch from eb352c0 to a28a316 Compare May 8, 2022 04:58
@utzcoz utzcoz changed the title Bump Robolectric to 4.8 Bump Robolectric to 4.8.1 May 28, 2022
@utzcoz utzcoz force-pushed the bump-Robolectric branch from a28a316 to 3b5a1f8 Compare May 28, 2022 12:22
@utzcoz utzcoz force-pushed the bump-Robolectric branch from 3b5a1f8 to c07d7b9 Compare June 17, 2022 14:15
utzcoz added 2 commits June 18, 2022 18:24
1. Bump sdk to 32.
2. Remove icu4j from testImplementation because Robolectric has a higher
   icu4j dependency.

Signed-off-by: utzcoz <[email protected]>
Before Robolectric 4.8, Surface#lockHardwareCanvas will throw
exception at PlatformViewWrapper#setTexture, because Robolectric
doesn't support to shadow Surface#lockHardwareCanvas, and it
uses real Android logic with native pointer address is 0.
This failure will ensure embeddedView's parent is null, because
PlatformViewsController#createForTextureLayer will fail because
of previous mentioned error, and PlatformViewsController#createForTextureLayer
will not add embeddedView to wrapperView. So this test can pass.
From Robolectric 4.8, it supports to shadow Surface#lockHardwareCanvas
and it can pass with default true valid value, and
PlatformViewsController#createForTextureLayer will run correctly
and add embeddedView to wrapperView, and initializePlatformViewIfNeeded
will fail because embeddedView's parent is not null. So adding a new shadow
class called ShadowReleasedSurface to simulate previous
Surface#lockHardwareCanvas failure to ensure this test can work with
Robolectric 4.8 and later versions. But it is just a workaround,
the root cause is this test case depends on just-failure behavior of
Surface#lockHardwareCanvas in old Robolectric.

Signed-off-by: utzcoz <[email protected]>
@utzcoz utzcoz force-pushed the bump-Robolectric branch from c07d7b9 to 6a20059 Compare June 18, 2022 11:26
@utzcoz utzcoz marked this pull request as ready for review June 18, 2022 11:27
@utzcoz
Copy link
Contributor Author

utzcoz commented Jun 18, 2022

Before Robolectric 4.8, Surface#lockHardwareCanvas will throw exception at PlatformViewWrapper#setTexture, because Robolectric doesn't support to shadow Surface#lockHardwareCanvas, and it uses real Android logic with native pointer address is 0. This failure will ensure embeddedView's parent is null, because PlatformViewsController#createForTextureLayer will fail because of previous mentioned error, and PlatformViewsController#createForTextureLayer will not add embeddedView to wrapperView. So this test can pass.

From Robolectric 4.8, it supports to shadow Surface#lockHardwareCanvas and it can pass with default true valid value, and
PlatformViewsController#createForTextureLayer will run correctly and add embeddedView to wrapperView, and initializePlatformViewIfNeeded will fail because embeddedView's parent is not null. So adding a new shadow class called ShadowReleasedSurface to simulate previous Surface#lockHardwareCanvas failure to ensure this test can work with Robolectric 4.8 and later versions. But it is just a workaround, the root cause is this test case depends on just-failure behavior of
Surface#lockHardwareCanvas in old Robolectric.

@utzcoz
Copy link
Contributor Author

utzcoz commented Jun 18, 2022

Hi @blasten , @jason-simmons , could you help to review this PR? Thanks.

@chinmaygarde chinmaygarde added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 23, 2022
@auto-submit auto-submit bot merged commit 2857531 into flutter:main Jun 23, 2022
jason-simmons added a commit to jason-simmons/flutter_engine that referenced this pull request Jun 23, 2022
jason-simmons added a commit that referenced this pull request Jun 23, 2022
@utzcoz utzcoz deleted the bump-Robolectric branch June 24, 2022 00:36
@utzcoz
Copy link
Contributor Author

utzcoz commented Jun 24, 2022

Hi @jason-simmons , I saw you added a new commit to revert it, does it cause new problem? I can try to fix it.

@jason-simmons
Copy link
Member

The Robolectric tests were failing in the CI environment

Example: https://ci.chromium.org/ui/p/flutter/builders/prod/Linux%20Android%20Debug%20Engine/16477/overview

@utzcoz
Copy link
Contributor Author

utzcoz commented Jun 24, 2022

Thanks. I will test it locally, and try to re-land it again.

@utzcoz utzcoz mentioned this pull request Jun 24, 2022
8 tasks
@utzcoz
Copy link
Contributor Author

utzcoz commented Jun 24, 2022

@jason-simmons The failed tests are added at e986f43 after my PR. I will try to resolve it compatibility on Robolectric 4.8.1.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants