Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

hasStrings Mac #20531

Merged
merged 30 commits into from
Sep 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
bb0f4be
hasStrings on mac
justinmc Aug 14, 2020
97acb46
WIP test
justinmc Aug 15, 2020
660092e
Clean up and remove test since unit testing on mac not set up?
justinmc Aug 27, 2020
ffc7af4
Test hasStrings
justinmc Aug 31, 2020
98adfc9
Test hasStrings after clearing the clipboard as well
justinmc Aug 31, 2020
59ad159
Run Mac unit tests in run_tests.py
justinmc Aug 31, 2020
084c926
Formatting
justinmc Aug 31, 2020
1a22b66
Build.gn formatting
justinmc Aug 31, 2020
67f0173
Test name
justinmc Sep 1, 2020
6177e74
WIP trying to get ocmock to work in Mac
justinmc Sep 2, 2020
24d28c8
Use new ocmock target coming in buildroot PR
justinmc Sep 3, 2020
51280f2
Mocked pasteboard working
justinmc Sep 3, 2020
a15798e
handleMethodCall removed from header and pasteboard moved to property…
justinmc Sep 3, 2020
878aa78
Docs clarification
justinmc Sep 3, 2020
c2bca18
clipboardHasStrings returns a bool, caller handles dictionary
justinmc Sep 3, 2020
54301f3
Split tests and some cleanup
justinmc Sep 3, 2020
70286fc
Deduplicate mocking code
justinmc Sep 3, 2020
6c778fe
Comment cleanup
justinmc Sep 3, 2020
b68a594
Analyzer fixes
justinmc Sep 3, 2020
25385b5
Merge branch 'master' into has-strings-mac
justinmc Sep 3, 2020
45b81b4
Reference new buildroot hash after my change to ocmock there
justinmc Sep 4, 2020
2d0728d
Merge branch 'master' into has-strings-mac
justinmc Sep 8, 2020
3c9ec6e
_pasteboard -> pasteboard
justinmc Sep 9, 2020
a52595d
Mock pasteboard value directly instead of mocking setString
justinmc Sep 9, 2020
d14a117
Don't run the mac tests with run_test.py, because they fail in AOT mode
justinmc Sep 9, 2020
3490d8a
Merge branch 'master' into has-strings-mac
justinmc Sep 9, 2020
e1566ac
pasteboard in the private header
justinmc Sep 10, 2020
491a585
Merge branch 'master' into has-strings-mac
justinmc Sep 11, 2020
8c2d240
Merge branch 'master' into has-strings-mac
justinmc Sep 11, 2020
06a6034
Use ocmock now that it has been made to work with mac and ocmock_src …
justinmc Sep 12, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,7 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextI
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/fixtures/flutter_desktop_test.dart
FILE: ../../../flutter/shell/platform/darwin/macos/framework/module.modulemap
Expand Down
6 changes: 5 additions & 1 deletion shell/platform/darwin/macos/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,10 @@ test_fixtures("flutter_desktop_darwin_fixtures") {
executable("flutter_desktop_darwin_unittests") {
testonly = true

sources = [ "framework/Source/FlutterEngineUnittests.mm" ]
sources = [
"framework/Source/FlutterEngineUnittests.mm",
"framework/Source/FlutterViewControllerTest.mm",
]

cflags_objcc = [ "-fobjc-arc" ]

Expand All @@ -114,6 +117,7 @@ executable("flutter_desktop_darwin_unittests") {
"//flutter/testing:dart",
"//flutter/testing:skia",
"//flutter/testing:testing_lib",
"//third_party/ocmock:ocmock",
]
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,11 @@ - (NSDictionary*)getClipboardData:(NSString*)format;
*/
- (void)setClipboardData:(NSDictionary*)data;

/**
* Returns true iff the clipboard contains nonempty string data.
*/
- (BOOL)clipboardHasStrings;

@end

#pragma mark - FlutterViewController implementation.
Expand Down Expand Up @@ -505,6 +510,8 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result {
} else if ([call.method isEqualToString:@"Clipboard.setData"]) {
[self setClipboardData:call.arguments];
result(nil);
} else if ([call.method isEqualToString:@"Clipboard.hasStrings"]) {
result(@{@"value" : @([self clipboardHasStrings])});
} else {
result(FlutterMethodNotImplemented);
}
Expand All @@ -517,7 +524,7 @@ - (void)playSystemSound:(NSString*)soundType {
}

- (NSDictionary*)getClipboardData:(NSString*)format {
NSPasteboard* pasteboard = [NSPasteboard generalPasteboard];
NSPasteboard* pasteboard = self.pasteboard;
if ([format isEqualToString:@(kTextPlainFormat)]) {
NSString* stringInPasteboard = [pasteboard stringForType:NSPasteboardTypeString];
return stringInPasteboard == nil ? nil : @{@"text" : stringInPasteboard};
Expand All @@ -526,14 +533,22 @@ - (NSDictionary*)getClipboardData:(NSString*)format {
}

- (void)setClipboardData:(NSDictionary*)data {
NSPasteboard* pasteboard = [NSPasteboard generalPasteboard];
NSPasteboard* pasteboard = self.pasteboard;
NSString* text = data[@"text"];
[pasteboard clearContents];
if (text && ![text isEqual:[NSNull null]]) {
[pasteboard clearContents];
[pasteboard setString:text forType:NSPasteboardTypeString];
}
}

- (BOOL)clipboardHasStrings {
return [self.pasteboard stringForType:NSPasteboardTypeString].length > 0;
}

- (NSPasteboard*)pasteboard {
return [NSPasteboard generalPasteboard];
}

#pragma mark - FlutterViewReshapeListener

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h"

#include "flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h"
#include "flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h"
#include "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h"
#include "flutter/testing/testing.h"
#import "third_party/ocmock/Source/OCMock/OCMock.h"

namespace flutter::testing {

// Returns a mock FlutterViewController that is able to work in environments
// without a real pasteboard.
id mockViewController(NSString* pasteboardString) {
NSString* fixtures = @(testing::GetFixturesPath());
FlutterDartProject* project = [[FlutterDartProject alloc]
initWithAssetsPath:fixtures
ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];

// 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 viewControllerMock = OCMPartialMock(viewController);
OCMStub([viewControllerMock pasteboard]).andReturn(pasteboardMock);
return viewControllerMock;
}

TEST(FlutterViewControllerTest, HasStringsWhenPasteboardEmpty) {
// Mock FlutterViewController so that it behaves like the pasteboard is empty.
id viewControllerMock = mockViewController(nil);

// Call hasStrings and expect it to be false.
__block bool calledAfterClear = false;
__block bool valueAfterClear;
FlutterResult resultAfterClear = ^(id result) {
calledAfterClear = true;
NSNumber* valueNumber = [result valueForKey:@"value"];
valueAfterClear = [valueNumber boolValue];
};
FlutterMethodCall* methodCallAfterClear =
[FlutterMethodCall methodCallWithMethodName:@"Clipboard.hasStrings" arguments:nil];
[viewControllerMock handleMethodCall:methodCallAfterClear result:resultAfterClear];
ASSERT_TRUE(calledAfterClear);
ASSERT_FALSE(valueAfterClear);
}

TEST(FlutterViewControllerTest, HasStringsWhenPasteboardFull) {
// Mock FlutterViewController so that it behaves like the pasteboard has a
// valid string.
id viewControllerMock = mockViewController(@"some string");

// Call hasStrings and expect it to be true.
__block bool called = false;
__block bool value;
FlutterResult result = ^(id result) {
called = true;
NSNumber* valueNumber = [result valueForKey:@"value"];
value = [valueNumber boolValue];
};
FlutterMethodCall* methodCall =
[FlutterMethodCall methodCallWithMethodName:@"Clipboard.hasStrings" arguments:nil];
[viewControllerMock handleMethodCall:methodCall result:result];
ASSERT_TRUE(called);
ASSERT_TRUE(value);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is doing a lot of different things; please make individual tests that are more targeted. You're already using a fake pasteboard, so there's no need to do the entire cycle in a single test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I split it into two and it's a lot cleaner.


} // flutter::testing
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
// The FlutterView for this view controller.
@property(nonatomic, readonly, nullable) FlutterView* flutterView;

/**
* This just returns the NSPasteboard so that it can be mocked in the tests.
*/
@property(nonatomic, readonly, nonnull) NSPasteboard* pasteboard;

/**
* Adds a responder for keyboard events. Key up and key down events are forwarded to all added
* responders.
Expand Down