-
Notifications
You must be signed in to change notification settings - Fork 6k
Eliminate android test log spam #44982
Eliminate android test log spam #44982
Conversation
…t in KeyboardManagerTest, PlatformViewsControllerTest and SinglePresentationViewTest
…fig consistant with targetApi lint annotation, bump itDescribesTextFieldsWithTextAndHint to 28 because test actually fails on api 26
… DartExecutor.send, InputMethodSubtype, and proof of concept migration from Roboletric.setupActivity
…nfig to match target api
…tionProvider.getApplicationContext() in SingleViewPresentationTest
…tric.setup/buildActivity, system ui flags
…tionProvider.getApplicationContext() in FlutterEngineGroupCacheTest
…c setup activity calls
refactor to use reflectors and direct following https://github.com/robolectric/robolectric/pull/6598/files as an example Unsure if SplashShadowResources is used anywhere
stuartmorgan-g
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.
Various small suggestions, but LGTM as a good step toward having fewer warnings!
| private FlutterActivityAndFragmentDelegate.Host mockHost; | ||
|
|
||
| @SuppressWarnings("deprecation") | ||
| // Robolectric.setupActivity |
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.
Consider extracting a Activity GetMockActivity() or similar that returns Robolectric.setupActivity(Activity.class) so you aren't suppressing the entire method.
...atform/android/test/io/flutter/embedding/android/FlutterActivityAndFragmentDelegateTest.java
Show resolved
Hide resolved
| verify(delegate, never()).detachFromFlutterEngine(); | ||
| } | ||
|
|
||
| @SuppressWarnings("deprecation") |
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.
Same.
| assertEquals(fragment.getExclusiveAppComponent(), delegate); | ||
| } | ||
|
|
||
| @SuppressWarnings("deprecation") |
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.
Same.
| import org.robolectric.util.reflector.ForType; | ||
|
|
||
| @SuppressWarnings("deprecation") | ||
| // getDrawableInt |
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.
Do you mean getDrawable(int)? There's no getDrawableInt in this code.
| } | ||
|
|
||
| @SuppressWarnings("deprecation") | ||
| // setMessageHandler is deprecated. |
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.
Is there an issue we can reference for this? If we have a test verifying that our own code is calling our own deprecated method, it seems like there's a problem somewhere.
| @TargetApi(33) | ||
| @Config(sdk = 33) | ||
| @SuppressWarnings("deprecation") | ||
| // DartExecutor.send is deprecated. |
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.
Can we reference an issue for all of these deprecated send calls (here and in other files)?
shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java
Outdated
Show resolved
Hide resolved
| return; | ||
| } | ||
|
|
||
| // Migrate to ActivityScenario by following https://github.com/robolectric/robolectric/pull/4736 |
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 looks like it should be a // TODO
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.
| gradle.projectsEvaluated { | ||
| tasks.withType(JavaCompile) { | ||
| options.compilerArgs << "-Xlint:deprecation" << "-Xmaxwarns" << "434" | ||
| options.compilerArgs << "-Xlint:deprecation" << "-Werror" << "-Xmaxwarns" << "434" |
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.
Do you want to include a TODO with an issue link for adding more lint options?
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.
…om buildActivity methods
…number of supressed warnings
…133211) flutter/engine@58dc868...27d75f6 2023-08-23 [email protected] Roll Skia from 76898dad9fda to a631fefdba37 (2 revisions) (flutter/engine#45027) 2023-08-23 [email protected] Revert "FontVariation.lerp, custom FontVariation constructors, and more documentation" (flutter/engine#45023) 2023-08-23 [email protected] [Impeller] Dat rvalue reference (fix engine head) (flutter/engine#45024) 2023-08-23 [email protected] Revert "Enable clang-tidy for pre-push (opt-out), exclude `performance-unnecessary-value-param`" (flutter/engine#45020) 2023-08-23 [email protected] Roll Skia from 4e42b51cfe27 to 76898dad9fda (1 revision) (flutter/engine#45019) 2023-08-23 [email protected] [Impeller] Add STB text backend. (flutter/engine#44887) 2023-08-23 [email protected] Followup to flutter/engine#44982 (flutter/engine#45018) 2023-08-23 [email protected] Roll Skia from 5428f147e632 to 4e42b51cfe27 (4 revisions) (flutter/engine#45016) 2023-08-23 [email protected] Eliminate android test log spam (flutter/engine#44982) 2023-08-23 [email protected] [web] Remove some unused functions (flutter/engine#44505) 2023-08-23 [email protected] Use decal TileMode in blur image_filter_test.dart (flutter/engine#45004) 2023-08-23 [email protected] FontVariation.lerp, custom FontVariation constructors, and more documentation (flutter/engine#44996) 2023-08-23 [email protected] [impeller] combine sampler and texture maps. (flutter/engine#44990) 2023-08-23 [email protected] [Impeller] Flutter GPU: Add HostBuffer. (flutter/engine#44696) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Set gradle to treat warnings as errors and suppress or fix all warnings in engine android tests. Fixes flutter/flutter/133070 After doing this work I was disappointed to realized that the only lint turned on was deprecration but this is still a step in the right direction. - Remove usages of deprecated junit.framework and replace with org.junit in KeyboardManagerTest, PlatformViewsControllerTest and SinglePresentationViewTest - Annotate deprecated usages of Registrar - Suppress warnings for getSystemWindowInsets and ensure roboletric config consistant with targetApi lint annotation, bump itDescribesTextFieldsWithTextAndHint to 28 because test actually fails on api 26 - Suppress warnings for DartExecutor.send - Suppress warnings for ClipboardManager.set/hasText - Suppress warnings for getWindowSystemUiVisibility, setMessageHandler, DartExecutor.send, InputMethodSubtype, and proof of concept migration from Roboletric.setupActivity - Suppress deprecation warnings, set minsdk on tests that were checking for sdk version - Suppress deprecation warnings in SpellCheckPluginTest - Suppress deprecation warnings in MouseCursorPluginTest, set minsdk config to match target api - Stop calling RuntimeEnvrionment.application and insted a call ApplicationProvider.getApplicationContext() in SingleViewPresentationTest - Start calling FlutterView(Context, FlutterSurfaceView) - Suppress deprecation warnings in PlatformPluginTest, getText, Robolectric.setup/buildActivity, system ui flags - Suppress deprecation warnings in PlayStoreDeferredComponentManagerTest - Suppress deprecation warnings in KeyboardChannelTest - Suppress deprecation warnings in SettingsChannelTest - Suppress deprecation warnings in ApplicationInfoLoaderTest - Stop calling RuntimeEnvrionment.application and insted a call ApplicationProvider.getApplicationContext() in FlutterEngineGroupCacheTest - Suppress deprecation warnings in FlutterAndroidComponentTest - Suppress deprecation warnings in FlutterFragmentTest, more robolectric setup activity calls - Suppress deprecation warnings in FlutterActivityAndFragmentDelegateTest - Shadow.directlyOn is incompatible with java 17+, refactor to use reflectors and direct following https://github.com/robolectric/robolectric/pull/6598/files as an example Unsure if SplashShadowResources is used anywhere - Enable warnings as errors - Formatting
One change was missing from flutter#44982 when I applied auto submit.
Set gradle to treat warnings as errors and suppress or fix all warnings in engine android tests.
Fixes flutter/flutter#133070
After doing this work I was disappointed to realized that the only lint turned on was deprecration but this is still a step in the right direction.
Pre-launch Checklist
///).