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

Commit 2a6f1a3

Browse files
authored
[macOS] Delete FlutterCompositor tests (#47527)
The tests for FlutterCompositor are not useful. The current tests test two things: 1. That the mocks we set up behave the way we set them up to behave. 2. That the implementation of FlutterCompositor is the current implementation of FlutterCompositor. As an example, consider FlutterCompositorTest.TestPresent: https://github.com/flutter/engine/blob/89e8de970cb99aa82e067bbdb4a8e927e53f0b28/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm#L107-L137 Ostensibly, this test verifies that the onPresent callback configured in our fake FlutterViewProvider implementation is called when FlutterCompositor::Present() is called. However, taking a look at the mocking setup: https://github.com/flutter/engine/blob/89e8de970cb99aa82e067bbdb4a8e927e53f0b28/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm#L47-L85 We do the following: 1. Mock out FlutterSurfaceManager such that when a surface is requested, we hand back a mock surface. A little gross since we're relying on some knowledge of implementation details of the compositor, but let's take this as reasonable for now. 2. We mock out `FlutterSurface asFlutterMetalTexture` to return a mock texture. Again, we're getting a bit deep into implementation details that the test shouldn't know about, but let's assume this gets us somewhere. 3. We mock out `FlutterSurfaceManager present:notify:` to actually call the `onPresent` callback if it's passed in. In effect, we're testing that: 1. We configured our mock for `FlutterSurfaceManager present:notify:` to call onPresent. 2. That `FlutterCompositor::Present` actually calls `FlutterSurfaceManager present:notify:` despite that being a simple implementation detail of that call. This removes these tests. I have filed the following issue to track refactoring this class for testability and adding tests: flutter/flutter#137648 Encountered these tests as part of deflaking and cleaning up memory allocations throughout the macOS desktop tests. Issue: flutter/flutter#137648 Issue: flutter/flutter#104789 Issue: flutter/flutter#127441 Issue: flutter/flutter#124840 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
1 parent 43ac4ac commit 2a6f1a3

File tree

4 files changed

+2
-142
lines changed

4 files changed

+2
-142
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4669,7 +4669,6 @@ ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCha
46694669
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterChannelKeyResponderTest.mm + ../../../flutter/LICENSE
46704670
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h + ../../../flutter/LICENSE
46714671
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm + ../../../flutter/LICENSE
4672-
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm + ../../../flutter/LICENSE
46734672
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject.mm + ../../../flutter/LICENSE
46744673
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h + ../../../flutter/LICENSE
46754674
ORIGIN: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderExternalTextureTest.mm + ../../../flutter/LICENSE
@@ -7473,7 +7472,6 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterChann
74737472
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterChannelKeyResponderTest.mm
74747473
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h
74757474
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.mm
7476-
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm
74777475
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject.mm
74787476
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h
74797477
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterEmbedderExternalTextureTest.mm

shell/platform/darwin/macos/BUILD.gn

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,6 @@ executable("flutter_desktop_darwin_unittests") {
173173
"framework/Source/FlutterAppDelegateTest.mm",
174174
"framework/Source/FlutterAppLifecycleDelegateTest.mm",
175175
"framework/Source/FlutterChannelKeyResponderTest.mm",
176-
"framework/Source/FlutterCompositorTest.mm",
177176
"framework/Source/FlutterEmbedderExternalTextureTest.mm",
178177
"framework/Source/FlutterEmbedderKeyResponderTest.mm",
179178
"framework/Source/FlutterEngineTest.mm",

shell/platform/darwin/macos/framework/Source/FlutterCompositor.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ namespace flutter {
2020
// FlutterCompositor creates and manages the backing stores used for
2121
// rendering Flutter content and presents Flutter content and Platform views.
2222
// Platform views are not yet supported.
23+
//
24+
// TODO(cbracken): refactor for testability. https://github.com/flutter/flutter/issues/137648
2325
class FlutterCompositor {
2426
public:
2527
// Create a FlutterCompositor with a view provider.

shell/platform/darwin/macos/framework/Source/FlutterCompositorTest.mm

Lines changed: 0 additions & 139 deletions
This file was deleted.

0 commit comments

Comments
 (0)