-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[Espresso] Create integration test and remove exemption #3367
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.
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
Conversation
There was a problem hiding this 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
|
There was already a file with the existing name so I added the word Integration to the file name. |
|
Hmm tests are failing with But this pr does not doe anything wtih multidex. I need to investigate but any suggestions are welcome. |
compilation error found by running ./gradlew app:assembleAndroidTest -Pverbose=true from packages/espresso/example
|
New test is passing locally with (from root) Any ideas are welcome. As an aside if you get "INSTALL_FAILED_VERIFICATION_FAILURE" then you can bypass it with |
|
I can reproduce the new failure by running |
…test app in build.gradle. Followed instructions https://developer.android.com/studio/build/configure-app-module#set-namespace to resolve warning about setting package id in build.gradle.
|
Test is timing out and I cant figure out why. |
|
Current plan is to debug by getting gcloud installed and trying to run Install instructions https://cloud.google.com/sdk/docs/install-sdk#mac |
|
local setup My version of the command. |
|
Unable to use gcloud to trigger builds normal google accounts are not authorized to run/modify/create projects or use existing ones. |
|
Running tests with |
|
|
|
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
|
Existing tests are failing and my new test is passing |
|
To locally reproduce local failures run |
|
Checking out main and running Give the following error 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
|
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 So perhaps those unit tests never actually worked in CI at all. Certainly not for the last several years. |
|
Further Evidence that the test does not run on main From an unrelated curris build |
gmackall
left a comment
There was a problem hiding this 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.
That's actually a build from a PR that only affected 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.) |
Sorry, I hadn't read the diff carefully enough; this comment of mine is completely wrong. The tests that don't work are 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. |
packages/espresso/CHANGELOG.md
Outdated
| @@ -1,3 +1,7 @@ | |||
| ## NEXT | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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()); |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Espresso] Create integration test and remove exemption
#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
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].CHANGELOG.mdto add a description of the change, [following repository CHANGELOG style].///).