Skip to content

[integration_test] Allow capture of screenshots for FlutterFragmentActivitys #132406

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.

Sign up for GitHub

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

Merged
auto-submit merged 21 commits into from
Sep 7, 2023
Merged

[integration_test] Allow capture of screenshots for FlutterFragmentActivitys #132406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
6dcfd40
potential fix for 89683
Aug 11, 2023
fefac8e
add unit tests, and fix a typo
Aug 11, 2023
0cdd710
Merge branch 'flutter:master' into integration_test_screenshot
gmackall Aug 11, 2023
b5e3df7
add missing newline at end of FlutterDeviceScreenshotTest.java
Aug 11, 2023
3745d11
modify ci to run android unit tests for integration_test
Aug 14, 2023
6da10e4
fix syntax for runCommand
Aug 14, 2023
57e5b74
try without workingDirectory
Aug 15, 2023
d6eed9d
Merge branch 'master' into integration_test_screenshot
gmackall Aug 15, 2023
95dc531
third time? is the charm for runCommandSyntax
Aug 15, 2023
d61e863
remove debugging pwd that shouldn't have been pushed in the first place
Aug 15, 2023
c70f7f2
debugging logs that should be removed
Aug 15, 2023
85e0df7
try generating gradlew
Aug 15, 2023
3be02a0
move debugging ls to after generation of gradlew
Aug 15, 2023
39d22b3
fix silly mistake in generation of gradlew
Aug 15, 2023
af37e60
upgrade to jdk 17
Aug 15, 2023
082836c
remove debugging ls
Aug 15, 2023
8ec9414
make command to run unit tests work on windows (gradlew vs gradlew.bat)
Aug 16, 2023
ad84bec
replace hack to generate gradlew with --config-only flag
Aug 21, 2023
6bcb3d5
fix comment for previous change
Aug 21, 2023
52b0ec9
Merge branch 'master' into integration_test_screenshot
gmackall Aug 21, 2023
365389e
adding package element back to manifest
Aug 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ targets:
{"dependency": "clang", "version": "git_revision:5d5aba78dbbee75508f01bcaa69aedb2ab79065a"},
{"dependency": "cmake", "version": "build_id:8787856497187628321"},
{"dependency": "ninja", "version": "version:1.9.0"},
{"dependency": "open_jdk", "version": "version:11"},
{"dependency": "open_jdk", "version": "version:17"},
{"dependency": "android_sdk", "version": "version:33v6"}
]
shard: framework_tests
Expand Down Expand Up @@ -3097,7 +3097,7 @@ targets:
[
{"dependency": "goldctl", "version": "git_revision:f808dcff91b221ae313e540c09d79696cd08b8de"},
{"dependency": "gems", "version": "v3.3.14"},
{"dependency": "open_jdk", "version": "version:11"},
{"dependency": "open_jdk", "version": "version:17"},
{"dependency": "android_sdk", "version": "version:33v6"}
]
shard: framework_tests
Expand Down Expand Up @@ -4470,7 +4470,7 @@ targets:
[
{"dependency": "goldctl", "version": "git_revision:f808dcff91b221ae313e540c09d79696cd08b8de"},
{"dependency": "vs_build", "version": "version:vs2019"},
{"dependency": "open_jdk", "version": "version:11"},
{"dependency": "open_jdk", "version": "version:17"},
{"dependency": "android_sdk", "version": "version:33v6"}
]
shard: framework_tests
Expand Down
17 changes: 17 additions & 0 deletions dev/bots/test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,23 @@ Future<void> _runFrameworkTests() async {
// Web-specific tests depend on Chromium, so they run as part of the web_long_running_tests shard.
'--exclude-tags=web',
]);
// Run java unit tests for integration_test
//
// Generate Gradle wrapper if it doesn't exist.
Process.runSync(
flutter,
<String>['build', 'apk', '--config-only'],
workingDirectory: path.join(flutterRoot, 'packages', 'integration_test', 'example', 'android'),
);
await runCommand(
path.join(flutterRoot, 'packages', 'integration_test', 'example', 'android', 'gradlew$bat'),
<String>[
':integration_test:testDebugUnitTest',
'--tests',
'dev.flutter.plugins.integration_test.FlutterDeviceScreenshotTest',
],
workingDirectory: path.join(flutterRoot, 'packages', 'integration_test', 'example', 'android'),
);
await _runFlutterTest(path.join(flutterRoot, 'packages', 'flutter_goldens'));
await _runFlutterTest(path.join(flutterRoot, 'packages', 'flutter_localizations'));
await _runFlutterTest(path.join(flutterRoot, 'packages', 'flutter_test'));
Expand Down
5 changes: 3 additions & 2 deletions packages/integration_test/android/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

group 'com.example.integration_test'
group 'dev.flutter.plugins.integration_test'
version '1.0-SNAPSHOT'

buildscript {
Expand Down Expand Up @@ -47,7 +47,8 @@ android {
dependencies {
// TODO(egarciad): These dependencies should not be added to release builds.
// https://github.com/flutter/flutter/issues/56591
api 'junit:junit:4.12'
testImplementation 'junit:junit:4.12'
testImplementation 'org.mockito:mockito-inline:5.0.0'

// https://developer.android.com/jetpack/androidx/releases/test/#1.2.0
api 'androidx.test:runner:1.2.0'
Expand Down
2 changes: 1 addition & 1 deletion packages/integration_test/android/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file. -->

<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="dev.flutter.integration_test">
package="dev.flutter.integration_test">
</manifest>
17 changes: 14 additions & 3 deletions ...t/android/src/main/java/dev/flutter/plugins/integration_test/FlutterDeviceScreenshot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@
import android.view.ViewGroup;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;

import io.flutter.embedding.android.FlutterActivity;
import io.flutter.embedding.android.FlutterFragment;
import io.flutter.embedding.android.FlutterFragmentActivity;
import io.flutter.embedding.android.FlutterSurfaceView;
import io.flutter.embedding.android.FlutterView;
import io.flutter.plugin.common.MethodChannel;
Expand Down Expand Up @@ -51,8 +55,15 @@ class FlutterDeviceScreenshot {
* @return the Flutter view.
*/
@Nullable
private static FlutterView getFlutterView(@NonNull Activity activity) {
return (FlutterView)activity.findViewById(FlutterActivity.FLUTTER_VIEW_ID);
@VisibleForTesting
public static FlutterView getFlutterView(@NonNull Activity activity) {
if (activity instanceof FlutterActivity) {
Copy link
Contributor

@reidbaker reidbaker Aug 21, 2023

Choose a reason for hiding this comment

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

Should this be ...

Suggested change
if (activity instanceof FlutterActivity) {
if (activity instanceof FlutterActivity ||| activity instanceOf FlutterFragmentActivity) {

Is it possible to avoid the instance of check at all?
go/UnhealthyIdioms#awkward-inheritance-hierarchy

Copy link
Member Author

@gmackall gmackall Aug 21, 2023

Choose a reason for hiding this comment

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

Should this be ...

No, note that the cases are indeed different here:
findViewById(FlutterActivity.FLUTTER_VIEW_ID) vs findViewById(FlutterFragment.FLUTTER_VIEW_ID) (notably, Flutter Activity vs Flutter Fragment

Copy link
Member Author

@gmackall gmackall Aug 21, 2023

Choose a reason for hiding this comment

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

I am not aware of a better way to do this without instanceof (I try to avoid it in general). Open to ideas if I'm missing something though.

We could just do something like (pseudo code)

if (<findbyviewid isn't null for FlutterActivity.FLUTTER_VIEW_ID>)
<return that>
if (<findbyviewid isn't null for FlutterFragmentFLUTTER_VIEW_ID>)
<return that>
<return null>

But if for whatever reason we are actually dealing with a FlutterFragmentActivity, and there is a view registered with the FlutterActivity.FLUTTER_VIEW_ID, then we would return the wrong view.

Copy link
Contributor

@reidbaker reidbaker Sep 1, 2023

Choose a reason for hiding this comment

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

After some further consideration if you want to move on feel free to file a follow-up ticket and merge this as is.

Copy link
Member Author

@gmackall gmackall Sep 7, 2023

Choose a reason for hiding this comment

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

#134252

return (FlutterView)activity.findViewById(FlutterActivity.FLUTTER_VIEW_ID);
} else if (activity instanceof FlutterFragmentActivity) {
return (FlutterView)activity.findViewById(FlutterFragment.FLUTTER_VIEW_ID);
} else {
return null;
}
}

/**
Expand Down Expand Up @@ -110,7 +121,7 @@ static void revertFlutterImage(@NonNull Activity activity) {
}
}

// Handlers use to capture a view.
// Handlers used to capture a view.
private static Handler backgroundHandler;
private static Handler mainHandler;

Expand Down
52 changes: 52 additions & 0 deletions ...droid/src/test/java/dev/flutter/plugins/integration_test/FlutterDeviceScreenshotTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package dev.flutter.plugins.integration_test;

import androidx.test.runner.AndroidJUnitRunner;

import org.junit.Test;

import static org.junit.Assert.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import android.app.Activity;

import io.flutter.embedding.android.FlutterActivity;
import io.flutter.embedding.android.FlutterFragment;
import io.flutter.embedding.android.FlutterFragmentActivity;
import io.flutter.embedding.android.FlutterView;

public class FlutterDeviceScreenshotTest extends AndroidJUnitRunner {
@Test
public void getFlutterView_returnsNullForNonFlutterActivity() {
Activity mockActivity = mock(Activity.class);
assertNull(FlutterDeviceScreenshot.getFlutterView(mockActivity));
}

@Test
public void getFlutterView_returnsFlutterViewForFlutterActivity() {
FlutterView mockFlutterView = mock(FlutterView.class);
FlutterActivity mockFlutterActivity = mock(FlutterActivity.class);
when(mockFlutterActivity.findViewById(FlutterActivity.FLUTTER_VIEW_ID))
.thenReturn(mockFlutterView);
assertEquals(
FlutterDeviceScreenshot.getFlutterView(mockFlutterActivity),
mockFlutterView
);
}

@Test
public void getFlutterView_returnsFlutterViewForFlutterFragmentActivity() {
FlutterView mockFlutterView = mock(FlutterView.class);
FlutterFragmentActivity mockFlutterFragmentActivity = mock(FlutterFragmentActivity.class);
when(mockFlutterFragmentActivity.findViewById(FlutterFragment.FLUTTER_VIEW_ID))
.thenReturn(mockFlutterView);
assertEquals(
FlutterDeviceScreenshot.getFlutterView(mockFlutterFragmentActivity),
mockFlutterView
);
}
}
5 changes: 5 additions & 0 deletions packages/integration_test/example/android/project-integration_test.lockfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ javax.activation:javax.activation-api:1.2.0=lintClassPath
javax.inject:javax.inject:1=debugAndroidTestCompileClasspath,debugAndroidTestRuntimeClasspath,debugCompileClasspath,debugRuntimeClasspath,debugUnitTestCompileClasspath,debugUnitTestRuntimeClasspath,lintClassPath,profileCompileClasspath,profileRuntimeClasspath,profileUnitTestCompileClasspath,profileUnitTestRuntimeClasspath,releaseCompileClasspath,releaseRuntimeClasspath,releaseUnitTestCompileClasspath,releaseUnitTestRuntimeClasspath
javax.xml.bind:jaxb-api:2.3.1=lintClassPath
junit:junit:4.12=debugAndroidTestCompileClasspath,debugAndroidTestRuntimeClasspath,debugCompileClasspath,debugRuntimeClasspath,debugUnitTestCompileClasspath,debugUnitTestRuntimeClasspath,profileCompileClasspath,profileRuntimeClasspath,profileUnitTestCompileClasspath,profileUnitTestRuntimeClasspath,releaseCompileClasspath,releaseRuntimeClasspath,releaseUnitTestCompileClasspath,releaseUnitTestRuntimeClasspath
net.bytebuddy:byte-buddy-agent:1.12.22=debugUnitTestCompileClasspath,debugUnitTestRuntimeClasspath,profileUnitTestCompileClasspath,profileUnitTestRuntimeClasspath,releaseUnitTestCompileClasspath,releaseUnitTestRuntimeClasspath
net.bytebuddy:byte-buddy:1.12.22=debugUnitTestCompileClasspath,debugUnitTestRuntimeClasspath,profileUnitTestCompileClasspath,profileUnitTestRuntimeClasspath,releaseUnitTestCompileClasspath,releaseUnitTestRuntimeClasspath
net.sf.jopt-simple:jopt-simple:4.9=lintClassPath
net.sf.kxml:kxml2:2.3.0=debugAndroidTestCompileClasspath,debugAndroidTestRuntimeClasspath,debugCompileClasspath,debugRuntimeClasspath,debugUnitTestCompileClasspath,debugUnitTestRuntimeClasspath,lintClassPath,profileCompileClasspath,profileRuntimeClasspath,profileUnitTestCompileClasspath,profileUnitTestRuntimeClasspath,releaseCompileClasspath,releaseRuntimeClasspath,releaseUnitTestCompileClasspath,releaseUnitTestRuntimeClasspath
org.apache.commons:commons-compress:1.12=lintClassPath
Expand Down Expand Up @@ -118,6 +120,9 @@ org.jetbrains.kotlinx:kotlinx-coroutines-core:1.5.2=debugAndroidTestCompileClass
org.jetbrains.trove4j:trove4j:20160824=lintClassPath
org.jetbrains:annotations:13.0=debugAndroidTestCompileClasspath,debugAndroidTestRuntimeClasspath,debugCompileClasspath,debugRuntimeClasspath,debugUnitTestCompileClasspath,debugUnitTestRuntimeClasspath,lintClassPath,profileCompileClasspath,profileRuntimeClasspath,profileUnitTestCompileClasspath,profileUnitTestRuntimeClasspath,releaseCompileClasspath,releaseRuntimeClasspath,releaseUnitTestCompileClasspath,releaseUnitTestRuntimeClasspath
org.jvnet.staxex:stax-ex:1.8=lintClassPath
org.mockito:mockito-core:5.0.0=debugUnitTestCompileClasspath,debugUnitTestRuntimeClasspath,profileUnitTestCompileClasspath,profileUnitTestRuntimeClasspath,releaseUnitTestCompileClasspath,releaseUnitTestRuntimeClasspath
org.mockito:mockito-inline:5.0.0=debugUnitTestCompileClasspath,debugUnitTestRuntimeClasspath,profileUnitTestCompileClasspath,profileUnitTestRuntimeClasspath,releaseUnitTestCompileClasspath,releaseUnitTestRuntimeClasspath
org.objenesis:objenesis:3.3=debugUnitTestRuntimeClasspath,profileUnitTestRuntimeClasspath,releaseUnitTestRuntimeClasspath
org.ow2.asm:asm-analysis:7.0=lintClassPath
org.ow2.asm:asm-analysis:9.2=androidJacocoAnt
org.ow2.asm:asm-commons:7.0=lintClassPath
Expand Down