Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
4 changes: 4 additions & 0 deletions packages/url_launcher/url_launcher_windows/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## NEXT

* Added unit tests.

## 2.0.0

* Migrate to null-safety.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ add_subdirectory(${FLUTTER_MANAGED_DIR})
# Application build
add_subdirectory("runner")

# Enable the test target.
set(include_url_launcher_windows_tests TRUE)

# Generated plugin build rules, which manage building the plugins and adding
# them to the application.
include(flutter/generated_plugins.cmake)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ add_custom_command(
${FLUTTER_TOOL_ENVIRONMENT}
"${FLUTTER_ROOT}/packages/flutter_tools/bin/tool_backend.bat"
windows-x64 $<CONFIG>
VERBATIM
)
add_custom_target(flutter_assemble DEPENDS
"${FLUTTER_LIBRARY}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@

#include "generated_plugin_registrant.h"

#include <url_launcher_windows/url_launcher_plugin.h>
#include <url_launcher_windows/url_launcher_windows.h>

void RegisterPlugins(flutter::PluginRegistry* registry) {
UrlLauncherPluginRegisterWithRegistrar(
registry->GetRegistrarForPlugin("UrlLauncherPlugin"));
UrlLauncherWindowsRegisterWithRegistrar(
registry->GetRegistrarForPlugin("UrlLauncherWindows"));
}
2 changes: 1 addition & 1 deletion packages/url_launcher/url_launcher_windows/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ flutter:
implements: url_launcher
platforms:
windows:
pluginClass: UrlLauncherPlugin
pluginClass: UrlLauncherWindows
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not great, but I don't have a solution I like better so far. The issue is that the flutter tool generates both an #include path and a call to the C API based on the value of pluginClass, so if it stays as FooPlugin, then the public header has to be called foo_plugin.h. But the plugin class's file should match its name, so should be called foo_plugin.h/cpp, and it's ugly to have two headers with the same name even if they are in different directories (and the implementation file of the C function would need to move, or have a name that doesn't match its header).

So the best option I can see is to have the pluginClass value not actually be the plugin class's name, but instead the plugin's name without the Plugin part (which isn't quite what I did here since there was already a mismatch with the federated name; in the template it would be Foo for a plugin whose implementation is FooPlugin). That should be harmless, but it's kind of confusing. We could deprecated pluginClass and replace it with a new key (continuing to honor pluginClass) to make it less odd, but I'm not sure it's worth the ecosystem migration (and/or inconsistency, depending on how many plugins migrate).

Copy link
Member

Choose a reason for hiding this comment

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

Hrm. Also not in love with this. Will book a quick sync to discuss, then we can summarise here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Summary: we agree that there's no obvious good option 😐 We're going to do this for now, and revisit our overall strategy when we update the plugin template to have the class in separate files. (And if we go a different route there, we can always update our plugins here.)


dependencies:
flutter:
Expand Down
55 changes: 52 additions & 3 deletions packages/url_launcher/url_launcher_windows/windows/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,20 @@ project(${PROJECT_NAME} LANGUAGES CXX)

set(PLUGIN_NAME "${PROJECT_NAME}_plugin")

add_library(${PLUGIN_NAME} SHARED
list(APPEND PLUGIN_SOURCES
"system_apis.cpp"
"system_apis.h"
"url_launcher_plugin.cpp"
"url_launcher_plugin.h"
)

add_library(${PLUGIN_NAME} SHARED
"include/url_launcher_windows/url_launcher_windows.h"
"url_launcher_windows.cpp"
${PLUGIN_SOURCES}
)
apply_standard_settings(${PLUGIN_NAME})
set_target_properties(${PLUGIN_NAME} PROPERTIES
CXX_VISIBILITY_PRESET hidden)
set_target_properties(${PLUGIN_NAME} PROPERTIES CXX_VISIBILITY_PRESET hidden)
target_compile_definitions(${PLUGIN_NAME} PRIVATE FLUTTER_PLUGIN_IMPL)
target_include_directories(${PLUGIN_NAME} INTERFACE
"${CMAKE_CURRENT_SOURCE_DIR}/include")
Expand All @@ -20,3 +28,44 @@ set(file_chooser_bundled_libraries
""
PARENT_SCOPE
)


# === Tests ===

if (${include_${PROJECT_NAME}_tests})
set(TEST_RUNNER "${PROJECT_NAME}_test")
enable_testing()
# TODO(stuartmorgan): Consider using a single shared, pre-checked-in googletest
# instance rather than downloading for each plugin. This approach makes sense
# for a template, but not for a monorepo with many plugins.
include(FetchContent)
FetchContent_Declare(
googletest
URL https://github.com/google/googletest/archive/release-1.11.0.zip
)
# Prevent overriding the parent project's compiler/linker settings
set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
# Disable install commands for gtest so it doesn't end up in the bundle.
set(INSTALL_GTEST OFF CACHE BOOL "Disable installation of googletest" FORCE)

FetchContent_MakeAvailable(googletest)

# The plugin's C API is not very useful for unit testing, so build the sources
# directly into the test binary rather than using the DLL.
add_executable(${TEST_RUNNER}
test/url_launcher_windows_test.cpp
${PLUGIN_SOURCES}
)
apply_standard_settings(${TEST_RUNNER})
target_include_directories(${TEST_RUNNER} PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}")
target_link_libraries(${TEST_RUNNER} PRIVATE flutter_wrapper_plugin)
target_link_libraries(${TEST_RUNNER} PRIVATE gtest_main gmock)
# flutter_wrapper_plugin has link dependencies on the Flutter DLL.
add_custom_command(TARGET ${TEST_RUNNER} POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy_if_different
"${FLUTTER_LIBRARY}" $<TARGET_FILE_DIR:${TEST_RUNNER}>
)

include(GoogleTest)
gtest_discover_tests(${TEST_RUNNER})
endif()
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
extern "C" {
#endif

FLUTTER_PLUGIN_EXPORT void UrlLauncherPluginRegisterWithRegistrar(
FLUTTER_PLUGIN_EXPORT void UrlLauncherWindowsRegisterWithRegistrar(
FlutterDesktopPluginRegistrarRef registrar);

#if defined(__cplusplus)
Expand Down
38 changes: 38 additions & 0 deletions packages/url_launcher/url_launcher_windows/windows/system_apis.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// 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.
#include "system_apis.h"

#include <windows.h>

namespace url_launcher_plugin {

SystemApis::SystemApis() {}

SystemApis::~SystemApis() {}

SystemApisImpl::SystemApisImpl() {}

SystemApisImpl::~SystemApisImpl() {}

LSTATUS SystemApisImpl::RegCloseKey(HKEY key) { return ::RegCloseKey(key); }

LSTATUS SystemApisImpl::RegOpenKeyExW(HKEY key, LPCWSTR sub_key, DWORD options,
REGSAM desired, PHKEY result) {
return ::RegOpenKeyExW(key, sub_key, options, desired, result);
}

LSTATUS SystemApisImpl::RegQueryValueExW(HKEY key, LPCWSTR value_name,
LPDWORD type, LPBYTE data,
LPDWORD data_size) {
return ::RegQueryValueExW(key, value_name, nullptr, type, data, data_size);
}

HINSTANCE SystemApisImpl::ShellExecuteW(HWND hwnd, LPCWSTR operation,
LPCWSTR file, LPCWSTR parameters,
LPCWSTR directory, int show_flags) {
return ::ShellExecuteW(hwnd, operation, file, parameters, directory,
show_flags);
}

} // namespace url_launcher_plugin
56 changes: 56 additions & 0 deletions packages/url_launcher/url_launcher_windows/windows/system_apis.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// 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.
#include <windows.h>

namespace url_launcher_plugin {

// An interface wrapping system APIs used by the plugin, for mocking.
class SystemApis {
public:
SystemApis();
virtual ~SystemApis();

// Disallow copy and move.
SystemApis(const SystemApis&) = delete;
SystemApis& operator=(const SystemApis&) = delete;

// Wrapper for RegCloseKey.
virtual LSTATUS RegCloseKey(HKEY key) = 0;

// Wrapper for RegQueryValueEx.
virtual LSTATUS RegQueryValueExW(HKEY key, LPCWSTR value_name, LPDWORD type,
LPBYTE data, LPDWORD data_size) = 0;

// Wrapper for RegOpenKeyEx.
virtual LSTATUS RegOpenKeyExW(HKEY key, LPCWSTR sub_key, DWORD options,
REGSAM desired, PHKEY result) = 0;

// Wrapper for ShellExecute.
virtual HINSTANCE ShellExecuteW(HWND hwnd, LPCWSTR operation, LPCWSTR file,
LPCWSTR parameters, LPCWSTR directory,
int show_flags) = 0;
};

// Implementation of SystemApis using the Win32 APIs.
class SystemApisImpl : public SystemApis {
public:
SystemApisImpl();
virtual ~SystemApisImpl();

// Disallow copy and move.
SystemApisImpl(const SystemApisImpl&) = delete;
SystemApisImpl& operator=(const SystemApisImpl&) = delete;

// SystemApis Implementation:
virtual LSTATUS RegCloseKey(HKEY key);
virtual LSTATUS RegOpenKeyExW(HKEY key, LPCWSTR sub_key, DWORD options,
REGSAM desired, PHKEY result);
virtual LSTATUS RegQueryValueExW(HKEY key, LPCWSTR value_name, LPDWORD type,
LPBYTE data, LPDWORD data_size);
virtual HINSTANCE ShellExecuteW(HWND hwnd, LPCWSTR operation, LPCWSTR file,
LPCWSTR parameters, LPCWSTR directory,
int show_flags);
};

} // namespace url_launcher_plugin
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
#include <flutter/method_call.h>
#include <flutter/method_result_functions.h>
#include <flutter/standard_method_codec.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <windows.h>

#include <memory>
#include <string>

#include "url_launcher_plugin.h"

namespace url_launcher_plugin {
namespace test {

namespace {

using flutter::EncodableMap;
using flutter::EncodableValue;
using ::testing::DoAll;
using ::testing::Pointee;
using ::testing::Return;
using ::testing::SetArgPointee;

class MockSystemApis : public SystemApis {
Copy link
Member

Choose a reason for hiding this comment

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

Nitpicky and up to you -- I wonder if a simple fake here that returns hardcoded/deterministic values and maybe a bool for open/close state would be enough, and make the tests themselves a bit more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The results can't be hard-coded, because I wanted to do full failure path testing, which means I would need a variable and a setter for the return value of each method. By the time I did that, the helper would be a lot more complicated than the mock declaration, so I came down on the side of gmock. (This also lets me check a few arguments to the calls trivially, which is a nice bit of added test robustness.)

public:
MOCK_METHOD(LSTATUS, RegCloseKey, (HKEY key), (override));
MOCK_METHOD(LSTATUS, RegQueryValueExW,
(HKEY key, LPCWSTR value_name, LPDWORD type, LPBYTE data,
LPDWORD data_size),
(override));
MOCK_METHOD(LSTATUS, RegOpenKeyExW,
(HKEY key, LPCWSTR sub_key, DWORD options, REGSAM desired,
PHKEY result),
(override));
MOCK_METHOD(HINSTANCE, ShellExecuteW,
(HWND hwnd, LPCWSTR operation, LPCWSTR file, LPCWSTR parameters,
LPCWSTR directory, int show_flags),
(override));
};

class MockMethodResult : public flutter::MethodResult<> {
public:
MOCK_METHOD(void, SuccessInternal, (const EncodableValue* result),
(override));
MOCK_METHOD(void, ErrorInternal,
(const std::string& error_code, const std::string& error_message,
const EncodableValue* details),
(override));
MOCK_METHOD(void, NotImplementedInternal, (), (override));
};

std::unique_ptr<EncodableValue> CreateArgumentsWithUrl(const std::string& url) {
EncodableMap args = {
{EncodableValue("url"), EncodableValue(url)},
};
return std::make_unique<EncodableValue>(args);
}

} // namespace

TEST(UrlLauncherPlugin, CanLaunchSuccessTrue) {
std::unique_ptr<MockSystemApis> system = std::make_unique<MockSystemApis>();
std::unique_ptr<MockMethodResult> result =
std::make_unique<MockMethodResult>();

// Return success values from the registery commands.
HKEY fake_key = reinterpret_cast<HKEY>(1);
EXPECT_CALL(*system, RegOpenKeyExW)
.WillOnce(DoAll(SetArgPointee<4>(fake_key), Return(ERROR_SUCCESS)));
EXPECT_CALL(*system, RegQueryValueExW).WillOnce(Return(ERROR_SUCCESS));
EXPECT_CALL(*system, RegCloseKey(fake_key)).WillOnce(Return(ERROR_SUCCESS));
// Expect a success response.
EXPECT_CALL(*result, SuccessInternal(Pointee(EncodableValue(true))));

UrlLauncherPlugin plugin(std::move(system));
plugin.HandleMethodCall(
flutter::MethodCall(
"canLaunch", CreateArgumentsWithUrl("https://some.url.com")

),
std::move(result));
}

TEST(UrlLauncherPlugin, CanLaunchQueryFailure) {
std::unique_ptr<MockSystemApis> system = std::make_unique<MockSystemApis>();
std::unique_ptr<MockMethodResult> result =
std::make_unique<MockMethodResult>();

// Return success values from the registery commands, except for the query,
// to simulate a scheme that is in the registry, but has no URL handler.
HKEY fake_key = reinterpret_cast<HKEY>(1);
EXPECT_CALL(*system, RegOpenKeyExW)
.WillOnce(DoAll(SetArgPointee<4>(fake_key), Return(ERROR_SUCCESS)));
EXPECT_CALL(*system, RegQueryValueExW).WillOnce(Return(ERROR_FILE_NOT_FOUND));
EXPECT_CALL(*system, RegCloseKey(fake_key)).WillOnce(Return(ERROR_SUCCESS));
// Expect a success response.
EXPECT_CALL(*result, SuccessInternal(Pointee(EncodableValue(false))));

UrlLauncherPlugin plugin(std::move(system));
plugin.HandleMethodCall(
flutter::MethodCall(
"canLaunch", CreateArgumentsWithUrl("https://some.url.com")

),
std::move(result));
}

TEST(UrlLauncherPlugin, CanLaunchHandlesOpenFailure) {
std::unique_ptr<MockSystemApis> system = std::make_unique<MockSystemApis>();
std::unique_ptr<MockMethodResult> result =
std::make_unique<MockMethodResult>();

// Return failure for opening.
EXPECT_CALL(*system, RegOpenKeyExW)
.WillOnce(Return(ERROR_BAD_PATHNAME));
// Expect a success response.
EXPECT_CALL(*result, SuccessInternal(Pointee(EncodableValue(false))));

UrlLauncherPlugin plugin(std::move(system));
plugin.HandleMethodCall(
flutter::MethodCall(
"canLaunch", CreateArgumentsWithUrl("https://some.url.com")

),
std::move(result));
}

TEST(UrlLauncherPlugin, LaunchSuccess) {
std::unique_ptr<MockSystemApis> system = std::make_unique<MockSystemApis>();
std::unique_ptr<MockMethodResult> result =
std::make_unique<MockMethodResult>();

// Return a success value (>32) from launching.
EXPECT_CALL(*system, ShellExecuteW)
.WillOnce(Return(reinterpret_cast<HINSTANCE>(33)));
// Expect a success response.
EXPECT_CALL(*result, SuccessInternal(Pointee(EncodableValue(true))));

UrlLauncherPlugin plugin(std::move(system));
plugin.HandleMethodCall(
flutter::MethodCall(
"launch", CreateArgumentsWithUrl("https://some.url.com")

),
std::move(result));
}

TEST(UrlLauncherPlugin, LaunchReportsFailure) {
std::unique_ptr<MockSystemApis> system = std::make_unique<MockSystemApis>();
std::unique_ptr<MockMethodResult> result =
std::make_unique<MockMethodResult>();

// Return a faile value (<=32) from launching.
EXPECT_CALL(*system, ShellExecuteW)
.WillOnce(Return(reinterpret_cast<HINSTANCE>(32)));
// Expect an error response.
EXPECT_CALL(*result, ErrorInternal);

UrlLauncherPlugin plugin(std::move(system));
plugin.HandleMethodCall(
flutter::MethodCall(
"launch", CreateArgumentsWithUrl("https://some.url.com")

),
std::move(result));
}

} // namespace test
} // namespace url_launcher_plugin
Loading