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

[Windows] Reduce warnings produced by unit tests #47724

Merged
auto-submit merged 3 commits into from
Nov 7, 2023
Merged

[Windows] Reduce warnings produced by unit tests #47724

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0769d11
[Windows] Reduce warnings produced by unit tests
loic-sharma Nov 6, 2023
c974af2
Accept >=1 calls for implementation detail mocks
loic-sharma Nov 7, 2023
8713d11
More ON_CALL -> EXPECT_CALL
loic-sharma Nov 7, 2023
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
Next Next commit
[Windows] Reduce warnings produced by unit tests
  • Loading branch information
@loic-sharma
loic-sharma committed Nov 6, 2023
commit 0769d119e77a3ffcc6dd588bff3aff43856d45d4
2 changes: 1 addition & 1 deletion shell/platform/windows/angle_surface_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class AngleSurfaceManager {

// Binds |egl_context_| to the current rendering thread and to the draw and
// read surfaces returning a boolean result reflecting success.
bool MakeCurrent();
virtual bool MakeCurrent();

// Unbinds the current EGL context from the current thread.
bool ClearCurrent();
Expand Down
60 changes: 33 additions & 27 deletions shell/platform/windows/flutter_window_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "gtest/gtest.h"

using testing::_;
using testing::AnyNumber;
using testing::Invoke;
using testing::Return;

Expand Down Expand Up @@ -133,8 +134,7 @@ TEST(FlutterWindowTest, OnBitmapSurfaceUpdated) {
// when the DPI scale is 100% (96 DPI).
TEST(FlutterWindowTest, OnCursorRectUpdatedRegularDPI) {
MockFlutterWindow win32window;
ON_CALL(win32window, GetDpiScale()).WillByDefault(Return(1.0));
EXPECT_CALL(win32window, GetDpiScale()).Times(1);
EXPECT_CALL(win32window, GetDpiScale()).WillOnce(Return(1.0));

Rect cursor_rect(Point(10, 20), Size(30, 40));
EXPECT_CALL(win32window, UpdateCursorRect(cursor_rect)).Times(1);
Expand All @@ -147,8 +147,7 @@ TEST(FlutterWindowTest, OnCursorRectUpdatedRegularDPI) {
// when the DPI scale is 150% (144 DPI).
TEST(FlutterWindowTest, OnCursorRectUpdatedHighDPI) {
MockFlutterWindow win32window;
ON_CALL(win32window, GetDpiScale()).WillByDefault(Return(1.5));
EXPECT_CALL(win32window, GetDpiScale()).Times(1);
EXPECT_CALL(win32window, GetDpiScale()).WillOnce(Return(1.5));

Rect expected_cursor_rect(Point(15, 30), Size(45, 60));
EXPECT_CALL(win32window, UpdateCursorRect(expected_cursor_rect)).Times(1);
Expand All @@ -160,7 +159,9 @@ TEST(FlutterWindowTest, OnCursorRectUpdatedHighDPI) {
TEST(FlutterWindowTest, OnPointerStarSendsDeviceType) {
FlutterWindow win32window(100, 100);
MockWindowBindingHandlerDelegate delegate;
EXPECT_CALL(delegate, OnWindowStateEvent).Times(AnyNumber());
win32window.SetView(&delegate);

// Move
EXPECT_CALL(delegate,
OnPointerMove(10.0, 10.0, kFlutterPointerDeviceKindMouse,
Expand Down Expand Up @@ -259,6 +260,7 @@ TEST(FlutterWindowTest, OnPointerStarSendsDeviceType) {
TEST(FlutterWindowTest, OnScrollCallsGetScrollOffsetMultiplier) {
MockFlutterWindow win32window;
MockWindowBindingHandlerDelegate delegate;
EXPECT_CALL(win32window, OnWindowStateEvent).Times(AnyNumber());
win32window.SetView(&delegate);

ON_CALL(win32window, GetScrollOffsetMultiplier())
Expand All @@ -277,6 +279,7 @@ TEST(FlutterWindowTest, OnScrollCallsGetScrollOffsetMultiplier) {
TEST(FlutterWindowTest, OnWindowRepaint) {
MockFlutterWindow win32window;
MockWindowBindingHandlerDelegate delegate;
EXPECT_CALL(win32window, OnWindowStateEvent).Times(AnyNumber());
win32window.SetView(&delegate);

EXPECT_CALL(delegate, OnWindowRepaint()).Times(1);
Expand All @@ -287,6 +290,7 @@ TEST(FlutterWindowTest, OnWindowRepaint) {
TEST(FlutterWindowTest, OnThemeChange) {
MockFlutterWindow win32window;
MockWindowBindingHandlerDelegate delegate;
EXPECT_CALL(win32window, OnWindowStateEvent).Times(AnyNumber());
win32window.SetView(&delegate);

EXPECT_CALL(delegate, OnHighContrastChanged).Times(1);
Expand All @@ -308,9 +312,9 @@ TEST(FlutterWindowTest, AccessibilityNodeWithoutView) {
TEST(FlutterWindowTest, AlertNode) {
std::unique_ptr<MockFlutterWindow> win32window =
std::make_unique<MockFlutterWindow>();
ON_CALL(*win32window, GetPlatformWindow()).WillByDefault(Return(nullptr));
ON_CALL(*win32window, GetAxFragmentRootDelegate())
.WillByDefault(Return(nullptr));
EXPECT_CALL(*win32window.get(), GetAxFragmentRootDelegate())
.WillOnce(Return(nullptr));
EXPECT_CALL(*win32window.get(), OnWindowStateEvent).Times(AnyNumber());
MockFlutterWindowsView view(std::move(win32window));
std::wstring message = L"Test alert";
EXPECT_CALL(view, NotifyWinEventWrapper(_, ax::mojom::Event::kAlert))
Expand All @@ -337,21 +341,22 @@ TEST(FlutterWindowTest, AlertNode) {

TEST(FlutterWindowTest, LifecycleFocusMessages) {
MockFlutterWindow win32window;
ON_CALL(win32window, GetPlatformWindow).WillByDefault([]() {
return reinterpret_cast<HWND>(1);
});
EXPECT_CALL(win32window, GetPlatformWindow)
.WillRepeatedly(Return(reinterpret_cast<HWND>(1)));
MockWindowBindingHandlerDelegate delegate;
win32window.SetView(&delegate);

WindowStateEvent last_event;
ON_CALL(delegate, OnWindowStateEvent)
.WillByDefault([&last_event](HWND hwnd, WindowStateEvent event) {
EXPECT_CALL(delegate, OnWindowStateEvent)
.WillRepeatedly([&last_event](HWND hwnd, WindowStateEvent event) {
last_event = event;
});
ON_CALL(win32window, OnWindowStateEvent)
.WillByDefault([&](WindowStateEvent event) {
EXPECT_CALL(win32window, OnWindowStateEvent)
.WillRepeatedly([&](WindowStateEvent event) {
win32window.FlutterWindow::OnWindowStateEvent(event);
});
EXPECT_CALL(win32window, OnResize).Times(AnyNumber());

win32window.SetView(&delegate);
Copy link
Member Author

@loic-sharma loic-sharma Nov 6, 2023

Choose a reason for hiding this comment

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

This was moved down as it also calls OnWindowStateEvent.


win32window.InjectWindowMessage(WM_SIZE, 0, 0);
EXPECT_EQ(last_event, WindowStateEvent::kHide);
Expand All @@ -368,13 +373,13 @@ TEST(FlutterWindowTest, LifecycleFocusMessages) {

TEST(FlutterWindowTest, CachedLifecycleMessage) {
MockFlutterWindow win32window;
ON_CALL(win32window, GetPlatformWindow).WillByDefault([]() {
return reinterpret_cast<HWND>(1);
});
ON_CALL(win32window, OnWindowStateEvent)
.WillByDefault([&](WindowStateEvent event) {
EXPECT_CALL(win32window, GetPlatformWindow)
.WillRepeatedly(Return(reinterpret_cast<HWND>(1)));
EXPECT_CALL(win32window, OnWindowStateEvent)
.WillRepeatedly([&](WindowStateEvent event) {
win32window.FlutterWindow::OnWindowStateEvent(event);
});
EXPECT_CALL(win32window, OnResize).Times(1);

// Restore
win32window.InjectWindowMessage(WM_SIZE, 0, MAKEWORD(1, 1));
Expand All @@ -385,8 +390,8 @@ TEST(FlutterWindowTest, CachedLifecycleMessage) {
MockWindowBindingHandlerDelegate delegate;
bool focused = false;
bool restored = false;
ON_CALL(delegate, OnWindowStateEvent)
.WillByDefault([&](HWND hwnd, WindowStateEvent event) {
EXPECT_CALL(delegate, OnWindowStateEvent)
.WillRepeatedly([&](HWND hwnd, WindowStateEvent event) {
if (event == WindowStateEvent::kFocus) {
focused = true;
} else if (event == WindowStateEvent::kShow) {
Expand All @@ -403,18 +408,19 @@ TEST(FlutterWindowTest, PosthumousWindowMessage) {
MockWindowBindingHandlerDelegate delegate;
int msg_count = 0;
HWND hwnd;
ON_CALL(delegate, OnWindowStateEvent)
.WillByDefault([&](HWND hwnd, WindowStateEvent event) { msg_count++; });
EXPECT_CALL(delegate, OnWindowStateEvent)
.WillRepeatedly([&](HWND hwnd, WindowStateEvent event) { msg_count++; });

{
MockFlutterWindow win32window(false);
ON_CALL(win32window, GetPlatformWindow).WillByDefault([&]() {
EXPECT_CALL(win32window, GetPlatformWindow).WillRepeatedly([&]() {
return win32window.FlutterWindow::GetPlatformWindow();
});
ON_CALL(win32window, OnWindowStateEvent)
.WillByDefault([&](WindowStateEvent event) {
EXPECT_CALL(win32window, OnWindowStateEvent)
.WillRepeatedly([&](WindowStateEvent event) {
win32window.FlutterWindow::OnWindowStateEvent(event);
});
EXPECT_CALL(win32window, OnResize).Times(AnyNumber());
win32window.SetView(&delegate);
win32window.InitializeChild("Title", 1, 1);
hwnd = win32window.GetPlatformWindow();
Expand Down
73 changes: 35 additions & 38 deletions shell/platform/windows/flutter_windows_engine_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -640,9 +640,8 @@ TEST_F(FlutterWindowsEngineTest, AlertPlatformMessage) {
std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
ui::AXPlatformNodeDelegateBase parent_delegate;
AlertPlatformNodeDelegate delegate(parent_delegate);
ON_CALL(*window_binding_handler, GetAlertDelegate).WillByDefault([&delegate] {
return &delegate;
});
EXPECT_CALL(*window_binding_handler, GetAlertDelegate)
.WillOnce(Return(&delegate));
Copy link
Member

@cbracken cbracken Nov 7, 2023

Choose a reason for hiding this comment

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

Similar question here and a couple other places below.

Copy link
Member Author

@loic-sharma loic-sharma Nov 7, 2023

Choose a reason for hiding this comment

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

Fixed!

MockFlutterWindowsView view(std::move(window_binding_handler));
view.SetEngine(engine.get());

Expand All @@ -660,9 +659,9 @@ TEST_F(FlutterWindowsEngineTest, AlertPlatformMessage) {
});

bool did_call = false;
ON_CALL(view, NotifyWinEventWrapper)
.WillByDefault([&did_call](ui::AXPlatformNodeWin* node,
ax::mojom::Event event) { did_call = true; });
EXPECT_CALL(view, NotifyWinEventWrapper)
.WillOnce([&did_call](ui::AXPlatformNodeWin* node,
ax::mojom::Event event) { did_call = true; });

engine->UpdateSemanticsEnabled(true);
engine->Run();
Expand Down Expand Up @@ -712,13 +711,13 @@ TEST_F(FlutterWindowsEngineTest, TestExit) {
EngineModifier modifier(engine.get());
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
ON_CALL(*handler, Quit)
.WillByDefault(
[&finished](std::optional<HWND> hwnd, std::optional<WPARAM> wparam,
std::optional<LPARAM> lparam,
UINT exit_code) { finished = exit_code == 0; });
ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([]() { return true; });
EXPECT_CALL(*handler, Quit).Times(1);
EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed));
EXPECT_CALL(*handler, Quit)
.WillOnce([&finished](std::optional<HWND> hwnd,
std::optional<WPARAM> wparam,
std::optional<LPARAM> lparam,
UINT exit_code) { finished = exit_code == 0; });
EXPECT_CALL(*handler, IsLastWindowOfProcess).WillOnce(Return(true));
modifier.SetLifecycleManager(std::move(handler));

engine->lifecycle_manager()->BeginProcessingExit();
Expand All @@ -738,7 +737,6 @@ TEST_F(FlutterWindowsEngineTest, TestExit) {
TEST_F(FlutterWindowsEngineTest, TestExitCancel) {
FlutterWindowsEngineBuilder builder{GetContext()};
builder.SetDartEntrypoint("exitTestCancel");
bool finished = false;
bool did_call = false;

auto engine = builder.Build();
Expand All @@ -750,12 +748,8 @@ TEST_F(FlutterWindowsEngineTest, TestExitCancel) {
EngineModifier modifier(engine.get());
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
ON_CALL(*handler, Quit)
.WillByDefault([&finished](std::optional<HWND> hwnd,
std::optional<WPARAM> wparam,
std::optional<LPARAM> lparam,
UINT exit_code) { finished = true; });
ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([]() { return true; });
EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed));
EXPECT_CALL(*handler, IsLastWindowOfProcess).WillOnce(Return(true));
EXPECT_CALL(*handler, Quit).Times(0);
modifier.SetLifecycleManager(std::move(handler));
engine->lifecycle_manager()->BeginProcessingExit();
Expand Down Expand Up @@ -783,10 +777,11 @@ TEST_F(FlutterWindowsEngineTest, TestExitCancel) {
while (!did_call) {
engine->task_runner()->ProcessTasks();
}

EXPECT_FALSE(finished);
}

// TODO(loicsharma): This test is passing incorrectly on the first
// WM_CLOSE message when instead it should pass on the second WM_CLOSE message.
// https://github.com/flutter/flutter/issues/137963
TEST_F(FlutterWindowsEngineTest, TestExitSecondCloseMessage) {
FlutterWindowsEngineBuilder builder{GetContext()};
builder.SetDartEntrypoint("exitTestExit");
Expand All @@ -801,18 +796,18 @@ TEST_F(FlutterWindowsEngineTest, TestExitSecondCloseMessage) {
EngineModifier modifier(engine.get());
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
auto& handler_obj = *handler;
ON_CALL(handler_obj, IsLastWindowOfProcess).WillByDefault([]() {
return true;
});
ON_CALL(handler_obj, Quit)
.WillByDefault(
[&handler_obj](std::optional<HWND> hwnd, std::optional<WPARAM> wparam,
EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed));
// TODO(loicsharma): These should be `EXPECT_CALL`s
// https://github.com/flutter/flutter/issues/137963
ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault(Return(true));
ON_CALL(*handler, Quit)
.WillByDefault([handler_ptr = handler.get()](
std::optional<HWND> hwnd, std::optional<WPARAM> wparam,
std::optional<LPARAM> lparam, UINT exit_code) {
handler_obj.WindowsLifecycleManager::Quit(hwnd, wparam, lparam,
exit_code);
});
ON_CALL(handler_obj, DispatchMessage)
handler_ptr->WindowsLifecycleManager::Quit(hwnd, wparam, lparam,
exit_code);
});
ON_CALL(*handler, DispatchMessage)
.WillByDefault(
[&engine](HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) {
engine->window_proc_delegate_manager()->OnTopLevelWindowProc(
Expand Down Expand Up @@ -863,7 +858,8 @@ TEST_F(FlutterWindowsEngineTest, TestExitCloseMultiWindow) {
EngineModifier modifier(engine.get());
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault([&finished]() {
EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed));
EXPECT_CALL(*handler, IsLastWindowOfProcess).WillOnce([&finished]() {
finished = true;
return false;
});
Expand Down Expand Up @@ -1065,8 +1061,8 @@ TEST_F(FlutterWindowsEngineTest, EnableLifecycleState) {
EngineModifier modifier(engine.get());
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
ON_CALL(*handler, SetLifecycleState)
.WillByDefault([handler_ptr = handler.get()](AppLifecycleState state) {
EXPECT_CALL(*handler, SetLifecycleState)
.WillRepeatedly([handler_ptr = handler.get()](AppLifecycleState state) {
handler_ptr->WindowsLifecycleManager::SetLifecycleState(state);
});
modifier.SetLifecycleManager(std::move(handler));
Expand Down Expand Up @@ -1118,8 +1114,8 @@ TEST_F(FlutterWindowsEngineTest, LifecycleStateToFrom) {
EngineModifier modifier(engine.get());
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
ON_CALL(*handler, SetLifecycleState)
.WillByDefault([handler_ptr = handler.get()](AppLifecycleState state) {
EXPECT_CALL(*handler, SetLifecycleState)
.WillRepeatedly([handler_ptr = handler.get()](AppLifecycleState state) {
handler_ptr->WindowsLifecycleManager::SetLifecycleState(state);
});
handler->begin_processing_callback = [&]() { enabled_lifecycle = true; };
Expand Down Expand Up @@ -1167,6 +1163,7 @@ TEST_F(FlutterWindowsEngineTest, ChannelListenedTo) {

bool lifecycle_began = false;
auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
EXPECT_CALL(*handler, SetLifecycleState).Times(1);
handler->begin_processing_callback = [&]() { lifecycle_began = true; };
modifier.SetLifecycleManager(std::move(handler));

Expand Down
3 changes: 3 additions & 0 deletions shell/platform/windows/flutter_windows_view_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ class MockAngleSurfaceManager : public AngleSurfaceManager {
(override));
MOCK_METHOD(void, DestroySurface, (), (override));

MOCK_METHOD(bool, MakeCurrent, (), (override));
MOCK_METHOD(void, SetVSyncEnabled, (bool), (override));

private:
Expand Down Expand Up @@ -1302,7 +1303,9 @@ TEST(FlutterWindowsViewTest, UpdatesVSyncOnDwmUpdates) {
FlutterWindowsView view(std::move(window_binding_handler));

InSequence s;
EXPECT_CALL(*surface_manager.get(), MakeCurrent).WillOnce(Return(true));
EXPECT_CALL(*surface_manager.get(), SetVSyncEnabled(true)).Times(1);
EXPECT_CALL(*surface_manager.get(), MakeCurrent).WillOnce(Return(true));
Copy link
Member Author

@loic-sharma loic-sharma Nov 6, 2023

Choose a reason for hiding this comment

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

This was making a GL context current, thereby causing other tests to complain that a GL context was unexpectedly current.

EXPECT_CALL(*surface_manager.get(), SetVSyncEnabled(false)).Times(1);

EXPECT_CALL(*engine.get(), Stop).Times(1);
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/windows/system_utils_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ TEST(SystemUtils, GetPreferredLanguageInfo) {

TEST(SystemUtils, GetPreferredLanguages) {
MockWindowsProcTable proc_table;
ON_CALL(proc_table, GetThreadPreferredUILanguages)
.WillByDefault(
EXPECT_CALL(proc_table, GetThreadPreferredUILanguages)
.WillRepeatedly(
[](DWORD flags, PULONG count, PZZWSTR languages, PULONG size) {
// Languages string ends in a double-null.
static const wchar_t lang[] = L"en-US\0";
Expand Down