Skip to content

Conversation

@reidbaker
Copy link
Contributor

@reidbaker reidbaker commented Mar 3, 2023

#12142

Update documentation for how exclusions for integration test works.
Add integration test to espresso and remove exclusion.
Update integration test to support multidex
Update Android Gradle Plugin to more recent version and update gradle wrapper to match.
Because of the update to AGP modify build.gradle to set namespace following the instructions found here https://developer.android.com/studio/build/configure-app-module#set-namespace.
Remove existing test that I believe is skipped in the existing tooling.

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 [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style].
  • 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.

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

Doh, it's been so long since I've done one of these setups that I led you astray; we forgot a critical step. The command I had you run to run it locally isn't what we use in CI on Android, unlike every other platform, because we use FTL to test Android. (I'm hoping we can get Android simulators up and running soon, in which case that command will be used for Android too.)

To enable it for FTL, you have to do the extra steps listed at https://github.com/flutter/flutter/tree/main/packages/integration_test#android-device-testing

…e/main/packages/integration_test\#android-device-testing but use a different file name because the suggested name was already taken by a unit test
@reidbaker
Copy link
Contributor Author

There was already a file with the existing name so I added the word Integration to the file name.

@reidbaker reidbaker requested a review from stuartmorgan-g March 9, 2023 19:50
@reidbaker
Copy link
Contributor Author

Hmm tests are failing with

/tmp/cirrus-ci-build/packages/espresso/example/android/app/src/main/java/io/flutter/app/FlutterMultiDexApplication.java:17: error: package androidx.multidex does not exist
import androidx.multidex.MultiDex;
                        ^
/tmp/cirrus-ci-build/packages/espresso/example/android/app/src/main/java/io/flutter/app/FlutterMultiDexApplication.java:25: error: cannot find symbol
    MultiDex.install(this);
    ^
  symbol:   variable MultiDex
  location: class FlutterMultiDexApplication
2 errors

FAILURE: Build failed with an exception.

But this pr does not doe anything wtih multidex. I need to investigate but any suggestions are welcome.

@reidbaker
Copy link
Contributor Author

New test is passing locally with (from root)dart run ./script/tool/bin/flutter_plugin_tools.dart drive-examples --android --packages espresso
from packages/expresso/example/android ./gradlew app:assembleAndroidTest -Pverbose=true
But is still failing in ci.

Any ideas are welcome.

As an aside if you get "INSTALL_FAILED_VERIFICATION_FAILURE" then you can bypass it with adb shell settings put global verifier_verify_adb_installs 0

@reidbaker
Copy link
Contributor Author

I can reproduce the new failure by running ./gradlew app:mergeDexDebug from packages/espresso/example/android

@reidbaker
Copy link
Contributor Author

@reidbaker
Copy link
Contributor Author

Current plan is to debug by getting gcloud installed and trying to run gcloud firebase test android run --type instrumentation --app build/app/outputs/apk/debug/app-debug.apk --test build/app/outputs/apk/androidTest/debug/app-debug-androidTest.apk --timeout 7m --results-bucket=gs://flutter_cirrus_testlab --results-dir=plugins_android_test/espresso/5839641094389760/d8d95d2c-8a7f-443d-886d-9b9aae3a2670/example/0/ --device model=redfin,version=30 --device model=starqlteue,version=26" or some variation of that command.

Install instructions https://cloud.google.com/sdk/docs/install-sdk#mac

@reidbaker
Copy link
Contributor Author

local setup

// Build apk 
packages/espresso/example 
flutter build apk
// Build Test 
packages/espresso/example/android 
./gradlew app:test

My version of the command.
gcloud firebase test android run --type instrumentation --app build/app/outputs/flutter-apk/app-release.apk --test build/app/outputs/apk/androidTest/debug/app-debug-androidTest.apk --timeout 7m --device model=starqlteue,version=26

@reidbaker
Copy link
Contributor Author

reidbaker commented Mar 14, 2023

Unable to use gcloud to trigger builds normal google accounts are not authorized to run/modify/create projects or use existing ones.
From flutter-infra chatroom the suggestion is to use led.
https://g3doc.corp.google.com/company/teams/flutter/infrastructure/playbook.md?cl=head#testing-repo-changes-with-led
https://chromium.googlesource.com/chromium/src/+/HEAD/docs/infra/using_led.md
Installed as part of depot_tools.

@reidbaker
Copy link
Contributor Author

reidbaker commented Mar 14, 2023

Running tests with flutter drive --driver test_driver/integration_test.dart --target integration_test/espresso_launch_test.dart and flutter test integration_test/espresso_launch_test.dart pass locally.

@reidbaker
Copy link
Contributor Author

gcloud firebase test android run --type instrumentation --app build/app/outputs/flutter-apk/app-debug.apk --test build/app/outputs/apk/androidTest/debug/app-debug-androidTest.apk --timeout 7m --device model=starqlteue,version=26 --project flutter-cirrus --results-bucket=gs://flutter_cirrus_testlab
Command started a test successfully.
https://firebase.corp.google.com/u/0/project/flutter-cirrus/testlab/histories/bh.61e29d9ae01b62d4/matrices/5019499127722619788

@reidbaker
Copy link
Contributor Author

Current theory is that if I follow https://github.com/flutter/flutter/wiki/Plugin-Tests#enabling-android-ui-tests in addition to the super call being made that this should pass.

… given in https://github.com/flutter/flutter/wiki/Plugin-Tests\#enabling-android-ui-tests,

The annotion is used by flutter_plugin_tools to run the correct subset of dart integration tests
@reidbaker
Copy link
Contributor Author

@reidbaker
Copy link
Contributor Author

https://firebase.corp.google.com/u/0/project/flutter-cirrus/testlab/histories/bh.61e29d9ae01b62d4/matrices/8496841808769326759/executions/bs.71460c71b724876d/logs

Existing tests are failing and my new test is passing

03-16 13:30:22.986: I/TestRunner(22457): started: placeholder test(com.example.espresso_example.MainActivityIntegrationTest)
03-16 13:30:22.989: I/flutter(22457): 00:00 +2: All tests passed!
03-16 13:30:22.990: I/MonitoringInstr(22457): Finishing activity: com.example.espresso_example.MainActivity@f0d6731
03-16 13:30:23.000: I/AdrenoGLES-0(1637): QUALCOMM build                   : 4783c89, I46ff5fc46f
03-16 13:30:23.000: I/AdrenoGLES-0(1637): Build Date                       : 11/30/20
03-16 13:30:23.000: I/AdrenoGLES-0(1637): OpenGL ES Shader Compiler Version: EV031.31.04.01
03-16 13:30:23.000: I/AdrenoGLES-0(1637): Local Branch                     : QPR2
03-16 13:30:23.000: I/AdrenoGLES-0(1637): Remote Branch                    : 
03-16 13:30:23.000: I/AdrenoGLES-0(1637): Remote Branch                    : 
03-16 13:30:23.000: I/AdrenoGLES-0(1637): Reconstruct Branch               : 
03-16 13:30:23.000: I/AdrenoGLES-0(1637): Build Config                     : S P 10.0.4 AArch64
03-16 13:30:23.000: I/AdrenoGLES-0(1637): Driver Path                      : /vendor/lib64/egl/libGLESv2_adreno.so
03-16 13:30:23.001: I/MonitoringInstr(22457): Unstopped activity count: 1
03-16 13:30:23.003: D/LifecycleMonitor(22457): Lifecycle status change: com.example.espresso_example.MainActivity@f0d6731 in: PAUSED

@reidbaker
Copy link
Contributor Author

To locally reproduce local failures run dart run ./script/tool/bin/flutter_plugin_tools.dart native-test --android --packages espresso from the root directory packages.

@reidbaker
Copy link
Contributor Author

Checking out main and running dart run ./script/tool/bin/flutter_plugin_tools.dart native-test --android --packages espresso

Give the following error


============================================================
|| Running for packages/espresso
============================================================

Running tests for Android...
----------------------------------------
No Android unit tests found for packages/espresso/example
No Android integration tests found for packages/espresso/example
No unit tests ran. Plugins are required to have unit tests.


The following packages had errors:
  packages/espresso:
    No unit tests ran (use --exclude if this is intentional).
See above for full details.

I currently believe that the tests that are failing were skipped before possibly because of the exclude rule.

…grationTest to MainActivityTest to match existing documentation
@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Mar 17, 2023

Ah, the reason that fails locally but not in CI is that they are also excluded because of script/config/exclude_native_unit_android.yaml. Looking at the blame in the other repo, when I first made having no unit tests an error in our tooling I excluded espresso under the (apparently mistaken) belief that it didn't need unit tests.

So perhaps those unit tests never actually worked in CI at all. Certainly not for the last several years.

@reidbaker
Copy link
Contributor Author

reidbaker commented Mar 17, 2023

Further Evidence that the test does not run on main

From an unrelated curris build
https://cirrus-ci.com/task/6359593459122176
shard 0 (just like my builds)
Expresso unit tests do not appear they way they do in my failing builds.

------------------------------------------------------------
  camera_android - 
ran
Ran for 1 package(s)
No issues found!

Copy link
Member

@gmackall gmackall left a comment

Choose a reason for hiding this comment

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

LGTM! Glad to see the issue of old tests breaking had a simple answer.

@stuartmorgan-g
Copy link
Collaborator

Further Evidence that the test does not run on main

From an unrelated curris build https://cirrus-ci.com/task/6359593459122176 shard 0 (just like my builds) Expresso unit tests do not appear they way they do in my failing builds.

------------------------------------------------------------
  camera_android - 
ran
Ran for 1 package(s)
No issues found!

That's actually a build from a PR that only affected camera_android. This is what the excluded test looks like.

This repo is pretty aggressive about skipping packages that can't be affected by the PR. The summary at the end includes skipped and excluded tests for auditability, so if a package isn't listed on the summary it wasn't even considered due to the change detection.

(This PR runs everything because you are changing script/config, which could affect anything.)

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Mar 17, 2023

Ah, the reason that fails locally but not in CI is that they are also excluded because of script/config/exclude_native_unit_android.yaml. Looking at the blame in the other repo, when I first made having no unit tests an error in our tooling I excluded espresso under the (apparently mistaken) belief that it didn't need unit tests.

So perhaps those unit tests never actually worked in CI at all. Certainly not for the last several years.

Sorry, I hadn't read the diff carefully enough; this comment of mine is completely wrong. The tests that don't work are androidTest tests, so are unrelated to the unit test exclusion. They are native integration tests. (The native unit test exclusion is intentional.)

So the fact that they weren't running is a mistake. Since plugins often don't need native integration tests, they don't require exclusions the way the standard test types do.

@@ -1,3 +1,7 @@
## NEXT
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be removed; tests are dev-only, so are exempt. You just need to add the label to override changelog detection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@Test
public void performTripleClick() {
WidgetInteraction interaction =
onFlutterWidget(withTooltip("Increment")).perform(click(), click()).perform(click());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you file an issue referencing this deleted code so we don't lose track of the existence of this native integration test? We should figure it why they are broken and fix them (just not in this PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reidbaker reidbaker added autosubmit Merge PR when tree becomes green via auto submit App override: no changelog needed Override the check requiring CHANGELOG updates for most changes labels Mar 20, 2023
@auto-submit auto-submit bot merged commit e4eb735 into flutter:main Mar 20, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 21, 2023
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
[Espresso] Create integration test and remove exemption
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App override: no changelog needed Override the check requiring CHANGELOG updates for most changes p: espresso platform-android

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants