From ee4fcf69dac8c162f33d77cf0520028ed71e8d12 Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Thu, 16 Nov 2023 10:13:50 -0800 Subject: [PATCH] [macOS] Replace pasteboard mock with fake (flutter/engine#48110) Also replaces the use of OCMock with a fake for tests involving the use of NSPasteboard. This avoids unnecessary use of OCMock in the tests, which has been responsible for flakiness in some tests, in particular where the mock is used across threads. This test was not problematic, but the fake makes the tests more readable. Issue: https://github.com/flutter/flutter/issues/104789 Issue: https://github.com/flutter/flutter/issues/127441 Issue: https://github.com/flutter/flutter/issues/124840 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style --- .../macos/framework/Source/FlutterEngine.mm | 31 +++++++--- .../Source/FlutterEngineTestUtils.mm | 60 +++++++++++++------ .../framework/Source/FlutterEngine_Internal.h | 11 +++- 3 files changed, 73 insertions(+), 29 deletions(-) diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index ac808395c0c..0232639b6cf 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -275,6 +275,24 @@ constexpr char kTextPlainFormat[] = "text/plain"; #pragma mark - +@implementation FlutterPasteboard + +- (NSInteger)clearContents { + return [[NSPasteboard generalPasteboard] clearContents]; +} + +- (NSString*)stringForType:(NSPasteboardType)dataType { + return [[NSPasteboard generalPasteboard] stringForType:dataType]; +} + +- (BOOL)setString:(nonnull NSString*)string forType:(nonnull NSPasteboardType)dataType { + return [[NSPasteboard generalPasteboard] setString:string forType:dataType]; +} + +@end + +#pragma mark - + /** * `FlutterPluginRegistrar` implementation handling a single plugin. */ @@ -452,6 +470,7 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi allowHeadlessExecution:(BOOL)allowHeadlessExecution { self = [super init]; NSAssert(self, @"Super init cannot be nil"); + _pasteboard = [[FlutterPasteboard alloc] init]; _active = NO; _visible = NO; _project = project ?: [[FlutterDartProject alloc] init]; @@ -1186,20 +1205,18 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi } - (NSDictionary*)getClipboardData:(NSString*)format { - NSPasteboard* pasteboard = self.pasteboard; if ([format isEqualToString:@(kTextPlainFormat)]) { - NSString* stringInPasteboard = [pasteboard stringForType:NSPasteboardTypeString]; + NSString* stringInPasteboard = [self.pasteboard stringForType:NSPasteboardTypeString]; return stringInPasteboard == nil ? nil : @{@"text" : stringInPasteboard}; } return nil; } - (void)setClipboardData:(NSDictionary*)data { - NSPasteboard* pasteboard = self.pasteboard; NSString* text = data[@"text"]; - [pasteboard clearContents]; + [self.pasteboard clearContents]; if (text && ![text isEqual:[NSNull null]]) { - [pasteboard setString:text forType:NSPasteboardTypeString]; + [self.pasteboard setString:text forType:NSPasteboardTypeString]; } } @@ -1207,10 +1224,6 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi return [self.pasteboard stringForType:NSPasteboardTypeString].length > 0; } -- (NSPasteboard*)pasteboard { - return [NSPasteboard generalPasteboard]; -} - - (std::vector)switches { return flutter::GetSwitchesFromEnvironment(); } diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.mm index 5e7ba44471e..b2ff26cab0d 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.mm @@ -4,11 +4,40 @@ // found in the LICENSE file. #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.h" + #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h" #include "flutter/testing/testing.h" +/** + * Fake pasteboard implementation to allow tests to work in environments without a real + * pasteboard. + */ +@interface FakePasteboard : FlutterPasteboard +@end + +@implementation FakePasteboard { + NSString* _result; +} + +- (NSInteger)clearContents { + size_t changeCount = (_result != nil) ? 1 : 0; + _result = nil; + return changeCount; +} + +- (NSString*)stringForType:(NSPasteboardType)dataType { + return _result; +} + +- (BOOL)setString:(NSString*)string forType:(NSPasteboardType)dataType { + _result = string; + return YES; +} + +@end + namespace flutter::testing { FlutterEngineTest::FlutterEngineTest() = default; @@ -45,26 +74,19 @@ void FlutterEngineTest::AddNativeCallback(const char* name, Dart_NativeFunction } id CreateMockFlutterEngine(NSString* pasteboardString) { - { - NSString* fixtures = @(testing::GetFixturesPath()); - FlutterDartProject* project = [[FlutterDartProject alloc] - initWithAssetsPath:fixtures - ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]]; - FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" - project:project - allowHeadlessExecution:true]; + NSString* fixtures = @(testing::GetFixturesPath()); + FlutterDartProject* project = [[FlutterDartProject alloc] + initWithAssetsPath:fixtures + ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" + project:project + allowHeadlessExecution:true]; - // Mock pasteboard so that this test will work in environments without a - // real pasteboard. - id pasteboardMock = OCMClassMock([NSPasteboard class]); - OCMExpect([pasteboardMock stringForType:[OCMArg any]]).andDo(^(NSInvocation* invocation) { - NSString* returnValue = pasteboardString.length > 0 ? pasteboardString : nil; - [invocation setReturnValue:&returnValue]; - }); - id engineMock = OCMPartialMock(engine); - OCMStub([engineMock pasteboard]).andReturn(pasteboardMock); - return engineMock; - } + FakePasteboard* pasteboardMock = [[FakePasteboard alloc] init]; + [pasteboardMock setString:pasteboardString forType:NSPasteboardTypeString]; + engine.pasteboard = pasteboardMock; + id engineMock = OCMPartialMock(engine); + return engineMock; } MockFlutterEngineTest::MockFlutterEngineTest() = default; diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h index 6b1a4e50e3c..eaac0d77965 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h @@ -66,6 +66,15 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { result:(nullable FlutterResult)result; @end +/** + * An NSPasteboard wrapper object to allow for substitution of a fake in unit tests. + */ +@interface FlutterPasteboard : NSObject +- (NSInteger)clearContents; +- (NSString*)stringForType:(NSPasteboardType)dataType; +- (BOOL)setString:(NSString*)string forType:(NSPasteboardType)dataType; +@end + @interface FlutterEngine () /** @@ -98,7 +107,7 @@ typedef NS_ENUM(NSInteger, FlutterAppExitResponse) { /** * This just returns the NSPasteboard so that it can be mocked in the tests. */ -@property(nonatomic, readonly, nonnull) NSPasteboard* pasteboard; +@property(nonatomic, nonnull) FlutterPasteboard* pasteboard; /** * The command line arguments array for the engine.