From 53cb8717b6c688c94795698b020135bc208eaa73 Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Wed, 15 Nov 2023 13:58:56 -0800 Subject: [PATCH] [macOS] Clean up allocations in menu plugin test Runs all FlutterMenuPlugin tests in an AutoReleasepoolTest, which ensures all allocations are cleaned up. Also extracts out some hackery into the FlutterMenuPluginTest fixture to ensure that NSApplication instantiates everything necessary to do menu bar manipulation. Also replaces the use of OCMock in FlutterMenuPluginTest.mm with a fake FakePluginRegistrar class. 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. Also fixes linter warnings about using NSLocalizedString for user-facing strings. Issue: https://github.com/flutter/flutter/issues/104789 Issue: https://github.com/flutter/flutter/issues/127441 Issue: https://github.com/flutter/flutter/issues/124840 --- .../framework/Source/FlutterMenuPluginTest.mm | 118 +++++++++++------- 1 file changed, 73 insertions(+), 45 deletions(-) diff --git a/shell/platform/darwin/macos/framework/Source/FlutterMenuPluginTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterMenuPluginTest.mm index 915df002c3d12..7d32208d93e3f 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterMenuPluginTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterMenuPluginTest.mm @@ -2,67 +2,103 @@ // 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/FlutterMenuPlugin.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterMenuPlugin_Internal.h" + +#import "flutter/shell/platform/common/platform_provided_menu.h" +#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterChannels.h" #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterPluginMacOS.h" #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterPluginRegistrarMacOS.h" #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterMenuPlugin.h" -#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterMenuPlugin_Internal.h" +#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h" - -#include "flutter/shell/platform/common/platform_provided_menu.h" +#include "flutter/testing/autoreleasepool_test.h" +#include "flutter/testing/testing.h" #include "gtest/gtest.h" -#import -#import "flutter/testing/testing.h" +@interface FakePluginRegistrar : NSObject +@property(nonatomic, readonly) id plugin; +@property(nonatomic, readonly) FlutterMethodChannel* channel; +@end + +@implementation FakePluginRegistrar +@synthesize messenger; +@synthesize textures; +@synthesize view; + +- (void)addMethodCallDelegate:(nonnull id)delegate + channel:(nonnull FlutterMethodChannel*)channel { + _plugin = delegate; + _channel = channel; + [_channel setMethodCallHandler:^(FlutterMethodCall* call, FlutterResult result) { + [delegate handleMethodCall:call result:result]; + }]; +} + +- (void)addApplicationDelegate:(nonnull NSObject*)delegate { +} + +- (void)registerViewFactory:(nonnull NSObject*)factory + withId:(nonnull NSString*)factoryId { +} -@interface FlutterMenuPluginTestObjc : NSObject -- (bool)testSetMenu; +- (void)publish:(nonnull NSObject*)value { +} + +- (nonnull NSString*)lookupKeyForAsset:(nonnull NSString*)asset { + return @""; +} + +- (nonnull NSString*)lookupKeyForAsset:(nonnull NSString*)asset + fromPackage:(nonnull NSString*)package { + return @""; +} @end -@implementation FlutterMenuPluginTestObjc +namespace flutter::testing { -- (bool)testSetMenu { - // Workaround to deflake the test. - // See: https://github.com/flutter/flutter/issues/104748#issuecomment-1159336728 - NSView* view = [[NSView alloc] initWithFrame:NSZeroRect]; - view.wantsLayer = YES; +// FlutterMenuPluginTest is an AutoreleasePoolTest that allocates an NSView. +// +// This supports the use of NSApplication features that rely on the assumption of a view, such as +// when modifying the application menu bar, or even accessing the NSApplication.localizedName +// property. +// +// See: https://github.com/flutter/flutter/issues/104748#issuecomment-1159336728 +class FlutterMenuPluginTest : public AutoreleasePoolTest { + public: + FlutterMenuPluginTest(); + ~FlutterMenuPluginTest() = default; + + private: + NSView* view_; +}; + +FlutterMenuPluginTest::FlutterMenuPluginTest() { + view_ = [[NSView alloc] initWithFrame:NSZeroRect]; + view_.wantsLayer = YES; +} +TEST_F(FlutterMenuPluginTest, TestSetMenu) { // Build a simulation of the default main menu. NSMenu* mainMenu = [[NSMenu alloc] init]; - NSMenuItem* appNameMenu = [[NSMenuItem alloc] initWithTitle:@"APP_NAME" + NSMenuItem* appNameMenu = [[NSMenuItem alloc] initWithTitle:NSLocalizedString(@"APP_NAME", nil) action:nil keyEquivalent:@""]; - NSMenu* submenu = [[NSMenu alloc] initWithTitle:@"Prexisting APP_NAME menu"]; - [submenu addItem:[[NSMenuItem alloc] initWithTitle:@"About APP_NAME" + NSMenu* submenu = + [[NSMenu alloc] initWithTitle:NSLocalizedString(@"Prexisting APP_NAME menu", nil)]; + [submenu addItem:[[NSMenuItem alloc] initWithTitle:NSLocalizedString(@"About APP_NAME", nil) action:nil keyEquivalent:@""]]; appNameMenu.submenu = submenu; [mainMenu addItem:appNameMenu]; [NSApp setMainMenu:mainMenu]; - id pluginRegistrarMock = - OCMProtocolMock(@protocol(FlutterPluginRegistrar)); - __block FlutterMethodChannel* pluginChannel; - __block FlutterMenuPlugin* plugin; - id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); - OCMStub([pluginRegistrarMock messenger]).andReturn(binaryMessengerMock); - OCMStub( // NOLINT(google-objc-avoid-throwing-exception) - [pluginRegistrarMock addMethodCallDelegate:[OCMArg any] channel:[OCMArg any]]) - .andDo(^(NSInvocation* invocation) { - id delegate; - FlutterMethodChannel* channel; - [invocation getArgument:&delegate atIndex:2]; - [invocation getArgument:&channel atIndex:3]; - pluginChannel = channel; - plugin = delegate; - [channel setMethodCallHandler:^(FlutterMethodCall* call, FlutterResult result) { - [delegate handleMethodCall:call result:result]; - }]; - }); - [FlutterMenuPlugin registerWithRegistrar:pluginRegistrarMock]; + FakePluginRegistrar* registrar = [[FakePluginRegistrar alloc] init]; + [FlutterMenuPlugin registerWithRegistrar:registrar]; + FlutterMenuPlugin* plugin = [registrar plugin]; NSDictionary* testMenus = @{ @"0" : @[ @@ -173,14 +209,6 @@ - (bool)testSetMenu { EXPECT_TRUE( [NSStringFromSelector([secondMenuLast action]) isEqualToString:@"flutterMenuItemSelected:"]); EXPECT_EQ([secondMenuLast tag], 7); - - return true; } -@end - -namespace flutter::testing { -TEST(FlutterMenuPluginTest, TestSetMenu) { - ASSERT_TRUE([[FlutterMenuPluginTestObjc alloc] testSetMenu]); -} } // namespace flutter::testing