-
Notifications
You must be signed in to change notification settings - Fork 6k
Made it so unit tests can be written against all ios engine code. #17624
Changes from all commits
b29dbaf
1472c5a
d9701f6
15347c5
d6e0f81
72abef8
b530f66
70afda8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,15 +37,12 @@ _flutter_framework_headers = [ | |
|
|
||
| _flutter_framework_headers_copy_dir = "$_flutter_framework_dir/Headers" | ||
|
|
||
| shared_library("create_flutter_framework_dylib") { | ||
| visibility = [ ":*" ] | ||
|
|
||
| output_name = "Flutter" | ||
|
|
||
| ldflags = [ "-Wl,-install_name,@rpath/Flutter.framework/Flutter" ] | ||
|
|
||
| public = _flutter_framework_headers | ||
| # TODO(54502): move this variable into //build/config/ios/ios_sdk.gni | ||
| # Version of iOS that we're targeting for tests. | ||
| ios_testing_deployment_target = "13.0" | ||
|
|
||
| source_set("flutter_framework_source") { | ||
| visibility = [ ":*" ] | ||
| cflags_objc = flutter_cflags_objc | ||
| cflags_objcc = flutter_cflags_objcc | ||
|
|
||
|
|
@@ -112,6 +109,21 @@ shared_library("create_flutter_framework_dylib") { | |
|
|
||
| sources += _flutter_framework_headers | ||
|
|
||
| defines = [ "FLUTTER_FRAMEWORK=1" ] | ||
|
|
||
| if (shell_enable_metal) { | ||
| defines += [ "FLUTTER_SHELL_ENABLE_METAL=1" ] | ||
|
|
||
| sources += [ | ||
| "ios_context_metal.h", | ||
| "ios_context_metal.mm", | ||
| "ios_external_texture_metal.h", | ||
| "ios_external_texture_metal.mm", | ||
| "ios_surface_metal.h", | ||
| "ios_surface_metal.mm", | ||
| ] | ||
| } | ||
|
|
||
| deps = [ | ||
| ":ios_gpu_configuration", | ||
| "//flutter/common", | ||
|
|
@@ -128,21 +140,6 @@ shared_library("create_flutter_framework_dylib") { | |
|
|
||
| public_configs = [ "//flutter:config" ] | ||
|
|
||
| defines = [ "FLUTTER_FRAMEWORK=1" ] | ||
|
|
||
| if (shell_enable_metal) { | ||
| defines += [ "FLUTTER_SHELL_ENABLE_METAL=1" ] | ||
|
|
||
| sources += [ | ||
| "ios_context_metal.h", | ||
| "ios_context_metal.mm", | ||
| "ios_external_texture_metal.h", | ||
| "ios_external_texture_metal.mm", | ||
| "ios_surface_metal.h", | ||
| "ios_surface_metal.mm", | ||
| ] | ||
| } | ||
|
|
||
| libs = [ | ||
| "CoreMedia.framework", | ||
| "CoreVideo.framework", | ||
|
|
@@ -153,6 +150,146 @@ shared_library("create_flutter_framework_dylib") { | |
| ] | ||
| } | ||
|
|
||
| ocmock_path = "../../../../../third_party/ocmock/Source" | ||
|
|
||
| # TODO(54503): Clone the OCMock repository so we can add a BUILD.gn to it. | ||
| static_library("ocmock") { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This definition goes in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, while you at it, make this a
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to be a static_library because it is linked by xcode's build system when building the testing bundle.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for clarifying.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wrt moving the build file. I added those details to the TODO about cloning the repository if we find we can't use chromium's clone. I'll follow up on this, like the other issue I'd like to do anything that involves an extra repo for another PR since i'm juggling this and LUCI already. |
||
| configs -= [ "//build/config/compiler:chromium_code" ] | ||
| cflags = [ | ||
| "-fvisibility=default", | ||
| "-mios-simulator-version-min=$ios_testing_deployment_target", | ||
| "-Wno-misleading-indentation", | ||
| ] | ||
| sources = [ | ||
| "$ocmock_path/OCMock/NSInvocation+OCMAdditions.h", | ||
| "$ocmock_path/OCMock/NSInvocation+OCMAdditions.m", | ||
| "$ocmock_path/OCMock/NSMethodSignature+OCMAdditions.h", | ||
| "$ocmock_path/OCMock/NSMethodSignature+OCMAdditions.m", | ||
| "$ocmock_path/OCMock/NSNotificationCenter+OCMAdditions.h", | ||
| "$ocmock_path/OCMock/NSNotificationCenter+OCMAdditions.m", | ||
| "$ocmock_path/OCMock/NSObject+OCMAdditions.h", | ||
| "$ocmock_path/OCMock/NSObject+OCMAdditions.m", | ||
| "$ocmock_path/OCMock/NSValue+OCMAdditions.h", | ||
| "$ocmock_path/OCMock/NSValue+OCMAdditions.m", | ||
| "$ocmock_path/OCMock/OCClassMockObject.h", | ||
| "$ocmock_path/OCMock/OCClassMockObject.m", | ||
| "$ocmock_path/OCMock/OCMArg.h", | ||
| "$ocmock_path/OCMock/OCMArg.m", | ||
| "$ocmock_path/OCMock/OCMArgAction.h", | ||
| "$ocmock_path/OCMock/OCMArgAction.m", | ||
| "$ocmock_path/OCMock/OCMBlockArgCaller.h", | ||
| "$ocmock_path/OCMock/OCMBlockArgCaller.m", | ||
| "$ocmock_path/OCMock/OCMBlockCaller.h", | ||
| "$ocmock_path/OCMock/OCMBlockCaller.m", | ||
| "$ocmock_path/OCMock/OCMBoxedReturnValueProvider.h", | ||
| "$ocmock_path/OCMock/OCMBoxedReturnValueProvider.m", | ||
| "$ocmock_path/OCMock/OCMConstraint.h", | ||
| "$ocmock_path/OCMock/OCMConstraint.m", | ||
| "$ocmock_path/OCMock/OCMExceptionReturnValueProvider.h", | ||
| "$ocmock_path/OCMock/OCMExceptionReturnValueProvider.m", | ||
| "$ocmock_path/OCMock/OCMExpectationRecorder.h", | ||
| "$ocmock_path/OCMock/OCMExpectationRecorder.m", | ||
| "$ocmock_path/OCMock/OCMFunctions.h", | ||
| "$ocmock_path/OCMock/OCMFunctions.m", | ||
| "$ocmock_path/OCMock/OCMFunctionsPrivate.h", | ||
| "$ocmock_path/OCMock/OCMIndirectReturnValueProvider.h", | ||
| "$ocmock_path/OCMock/OCMIndirectReturnValueProvider.m", | ||
| "$ocmock_path/OCMock/OCMInvocationExpectation.h", | ||
| "$ocmock_path/OCMock/OCMInvocationExpectation.m", | ||
| "$ocmock_path/OCMock/OCMInvocationMatcher.h", | ||
| "$ocmock_path/OCMock/OCMInvocationMatcher.m", | ||
| "$ocmock_path/OCMock/OCMInvocationStub.h", | ||
| "$ocmock_path/OCMock/OCMInvocationStub.m", | ||
| "$ocmock_path/OCMock/OCMLocation.h", | ||
| "$ocmock_path/OCMock/OCMLocation.m", | ||
| "$ocmock_path/OCMock/OCMMacroState.h", | ||
| "$ocmock_path/OCMock/OCMMacroState.m", | ||
| "$ocmock_path/OCMock/OCMNotificationPoster.h", | ||
| "$ocmock_path/OCMock/OCMNotificationPoster.m", | ||
| "$ocmock_path/OCMock/OCMObserverRecorder.h", | ||
| "$ocmock_path/OCMock/OCMObserverRecorder.m", | ||
| "$ocmock_path/OCMock/OCMPassByRefSetter.h", | ||
| "$ocmock_path/OCMock/OCMPassByRefSetter.m", | ||
| "$ocmock_path/OCMock/OCMRealObjectForwarder.h", | ||
| "$ocmock_path/OCMock/OCMRealObjectForwarder.m", | ||
| "$ocmock_path/OCMock/OCMRecorder.h", | ||
| "$ocmock_path/OCMock/OCMRecorder.m", | ||
| "$ocmock_path/OCMock/OCMReturnValueProvider.h", | ||
| "$ocmock_path/OCMock/OCMReturnValueProvider.m", | ||
| "$ocmock_path/OCMock/OCMStubRecorder.h", | ||
| "$ocmock_path/OCMock/OCMStubRecorder.m", | ||
| "$ocmock_path/OCMock/OCMVerifier.h", | ||
| "$ocmock_path/OCMock/OCMVerifier.m", | ||
| "$ocmock_path/OCMock/OCMock.h", | ||
| "$ocmock_path/OCMock/OCMockObject.h", | ||
| "$ocmock_path/OCMock/OCMockObject.m", | ||
| "$ocmock_path/OCMock/OCObserverMockObject.h", | ||
| "$ocmock_path/OCMock/OCObserverMockObject.m", | ||
| "$ocmock_path/OCMock/OCPartialMockObject.h", | ||
| "$ocmock_path/OCMock/OCPartialMockObject.m", | ||
| "$ocmock_path/OCMock/OCProtocolMockObject.h", | ||
| "$ocmock_path/OCMock/OCProtocolMockObject.m", | ||
| ] | ||
| include_dirs = [ "$ocmock_path" ] | ||
| } | ||
|
|
||
| ios_test_flutter_path = rebase_path("$root_out_dir/libios_test_flutter.dylib") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps still mention that it's for simulators in the name
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added comment |
||
| platform_frameworks_path = "$ios_sdk_path/../../Library/Frameworks/" | ||
|
|
||
| # NOTE: This currently only supports simulator targets because of the install_name. | ||
| # TODO(54504): Switch the install_name and make the test runner copy the dynamic | ||
| # library into the testing bundle. | ||
| shared_library("ios_test_flutter") { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be a good idea now to track decommissioning the unused
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That could be useful in the future if someone wanted to do the work of moving the whole build system over to GN. This addition doesn't necessarily obsolesce target. I'll add a note. I'm afraid to rip out something I don't have to touch. |
||
| visibility = [ ":*" ] | ||
| cflags = [ | ||
| "-fvisibility=default", | ||
| "-F$platform_frameworks_path", | ||
| "-fobjc-arc", | ||
| "-mios-simulator-version-min=$ios_testing_deployment_target", | ||
| ] | ||
| ldflags = [ | ||
| "-F$platform_frameworks_path", | ||
| "-Wl,-framework,XCTest", | ||
| "-Wl,-install_name,$ios_test_flutter_path", | ||
| ] | ||
| configs -= [ | ||
| "//build/config/gcc:symbol_visibility_hidden", | ||
| "//build/config:symbol_visibility_hidden", | ||
| ] | ||
| sources = [ | ||
| "framework/Source/FlutterBinaryMessengerRelayTest.mm", | ||
| "framework/Source/FlutterEngineTest.mm", | ||
| "framework/Source/FlutterPluginAppLifeCycleDelegateTest.m", | ||
| "framework/Source/FlutterTextInputPluginTest.m", | ||
| "framework/Source/FlutterViewControllerTest.mm", | ||
| "framework/Source/SemanticsObjectTest.mm", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if there's anything we can do to make sure people don't forget to add to this when creating new files since it could be easy to miss. We should at least have a testing/ios/IosUnitTests/README.md.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added a readme |
||
| ] | ||
| deps = [ | ||
| ":flutter_framework_source", | ||
| ":ocmock", | ||
| "//flutter/shell/platform/darwin/common:framework_shared", | ||
| "//third_party/skia", | ||
| ] | ||
| include_dirs = [ "$ocmock_path" ] | ||
| public_configs = [ "//flutter:config" ] | ||
| } | ||
|
|
||
| shared_library("create_flutter_framework_dylib") { | ||
| visibility = [ ":*" ] | ||
|
|
||
| output_name = "Flutter" | ||
|
|
||
| ldflags = [ "-Wl,-install_name,@rpath/Flutter.framework/Flutter" ] | ||
|
|
||
| public = _flutter_framework_headers | ||
|
|
||
| deps = [ | ||
| ":flutter_framework_source", | ||
| ] | ||
|
|
||
| public_configs = [ "//flutter:config" ] | ||
| } | ||
|
|
||
| copy("copy_dylib") { | ||
| visibility = [ ":*" ] | ||
|
|
||
|
|
@@ -257,6 +394,7 @@ test_fixtures("flutter_tests_fixtures") { | |
| fixtures = [] | ||
| } | ||
|
|
||
| # Note: This currently isn't used, it might be removed. | ||
| ios_app("FlutterTests") { | ||
| testonly = true | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -122,7 +122,9 @@ - (void)testItReportsPlatformBrightnessWhenViewWillAppear { | |
| } | ||
|
|
||
| - (void)testItReportsDarkPlatformBrightnessWhenTraitCollectionRequestsIt { | ||
| if (!@available(iOS 13, *)) { | ||
| if (@available(iOS 13, *)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. How did we miss this earlier? I expect clang had warnings for these.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "-Werror" isn't on by default in xcode projects, we were previously using xcode to build them. |
||
| // noop | ||
| } else { | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -171,7 +173,9 @@ - (UITraitCollection*)fakeTraitCollectionWithUserInterfaceStyle:(UIUserInterface | |
| #pragma mark - Platform Contrast | ||
|
|
||
| - (void)testItReportsNormalPlatformContrastByDefault { | ||
| if (!@available(iOS 13, *)) { | ||
| if (@available(iOS 13, *)) { | ||
| // noop | ||
| } else { | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -199,7 +203,9 @@ - (void)testItReportsNormalPlatformContrastByDefault { | |
| } | ||
|
|
||
| - (void)testItReportsPlatformContrastWhenViewWillAppear { | ||
| if (!@available(iOS 13, *)) { | ||
| if (@available(iOS 13, *)) { | ||
| // noop | ||
| } else { | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -227,7 +233,9 @@ - (void)testItReportsPlatformContrastWhenViewWillAppear { | |
| } | ||
|
|
||
| - (void)testItReportsHighContrastWhenTraitCollectionRequestsIt { | ||
| if (!@available(iOS 13, *)) { | ||
| if (@available(iOS 13, *)) { | ||
| // noop | ||
| } else { | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -453,13 +461,12 @@ - (void)testWillDeallocNotification { | |
| FlutterViewController* realVC = [[FlutterViewController alloc] initWithEngine:engine | ||
| nibName:nil | ||
| bundle:nil]; | ||
| id observer = | ||
| [[NSNotificationCenter defaultCenter] addObserverForName:FlutterViewControllerWillDealloc | ||
| object:nil | ||
| queue:[NSOperationQueue mainQueue] | ||
| usingBlock:^(NSNotification* _Nonnull note) { | ||
| [expectation fulfill]; | ||
| }]; | ||
| [[NSNotificationCenter defaultCenter] addObserverForName:FlutterViewControllerWillDealloc | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use https://developer.apple.com/documentation/xctest/xctestcase/1500964-expectationfornotification?language=objc to fulfill on notifications. |
||
| object:nil | ||
| queue:[NSOperationQueue mainQueue] | ||
| usingBlock:^(NSNotification* _Nonnull note) { | ||
| [expectation fulfill]; | ||
| }]; | ||
| realVC = nil; | ||
| } | ||
| [self waitForExpectations:@[ expectation ] timeout:1.0]; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| #import <OCMock/OCMock.h> | ||
| #import <XCTest/XCTest.h> | ||
|
|
||
| #include "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" | ||
| #import "flutter/shell/platform/darwin/ios/framework/Source/SemanticsObject.h" | ||
|
|
||
| FLUTTER_ASSERT_ARC | ||
|
|
||
| namespace flutter { | ||
| namespace { | ||
| class MockAccessibilityBridge : public AccessibilityBridgeIos { | ||
| public: | ||
| MockAccessibilityBridge() { view_ = [[UIView alloc] init]; } | ||
| UIView* view() const override { return view_; } | ||
| UIView<UITextInput>* textInputView() override { return nil; } | ||
| void DispatchSemanticsAction(int32_t id, SemanticsAction action) override {} | ||
| void DispatchSemanticsAction(int32_t id, | ||
| SemanticsAction action, | ||
| std::vector<uint8_t> args) override {} | ||
| FlutterPlatformViewsController* GetPlatformViewsController() const override { return nil; } | ||
|
|
||
| private: | ||
| UIView* view_; | ||
| }; | ||
| } // namespace | ||
| } // namespace flutter | ||
|
|
||
| @interface SemanticsObjectTest : XCTestCase | ||
| @end | ||
|
|
||
| @implementation SemanticsObjectTest | ||
|
|
||
| - (void)testCreate { | ||
| fml::WeakPtrFactory<flutter::AccessibilityBridgeIos> factory( | ||
| new flutter::MockAccessibilityBridge()); | ||
| fml::WeakPtr<flutter::AccessibilityBridgeIos> bridge = factory.GetWeakPtr(); | ||
| SemanticsObject* object = [[SemanticsObject alloc] initWithBridge:bridge uid:0]; | ||
| XCTAssertNotNil(object); | ||
| } | ||
|
|
||
| @end |
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.
we can add a non-public visibility clause here to be clean
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