From d5b2969546dfa1e7cc017b332e02a68b324c3814 Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Wed, 16 Sep 2015 10:58:34 -0700 Subject: [PATCH] Simplify TestRunner now that we don't have test_sky --- services/sky/internals.cc | 4 - sky/engine/bindings/sky_internals.dart | 2 - sky/packages/sky_services/BUILD.gn | 1 - sky/services/testing/BUILD.gn | 17 ---- sky/services/testing/test_harness.mojom | 13 --- sky/shell/BUILD.gn | 2 - sky/shell/linux/main_linux.cc | 7 +- .../linux/platform_service_provider_linux.cc | 4 - sky/shell/mac/main_mac.mm | 6 +- .../mac/platform_service_provider_mac.cc | 6 -- sky/shell/testing/test_runner.cc | 92 +------------------ sky/shell/testing/test_runner.h | 30 +----- sky/shell/testing/testing.cc | 26 ++---- sky/shell/testing/testing.h | 2 +- sky/shell/ui/internals.cc | 4 - 15 files changed, 28 insertions(+), 188 deletions(-) delete mode 100644 sky/services/testing/BUILD.gn delete mode 100644 sky/services/testing/test_harness.mojom diff --git a/services/sky/internals.cc b/services/sky/internals.cc index 3dbd5dfbde8..b6046fc64b0 100644 --- a/services/sky/internals.cc +++ b/services/sky/internals.cc @@ -27,9 +27,6 @@ Internals* GetInternals() { return static_cast(state->GetUserData(&kInternalsKey)); } -void NotifyTestComplete(Dart_NativeArguments args) { -} - void TakeRootBundleHandle(Dart_NativeArguments args) { Dart_SetIntegerReturnValue(args, GetInternals()->TakeRootBundleHandle().value()); @@ -56,7 +53,6 @@ void TakeServiceRegistry(Dart_NativeArguments args) { } const DartBuiltin::Natives kNativeFunctions[] = { - {"notifyTestComplete", NotifyTestComplete, 1}, {"takeRootBundleHandle", TakeRootBundleHandle, 0}, {"takeServiceRegistry", TakeServiceRegistry, 0}, {"takeServicesProvidedByEmbedder", TakeServicesProvidedByEmbedder, 0}, diff --git a/sky/engine/bindings/sky_internals.dart b/sky/engine/bindings/sky_internals.dart index c4e7fe49d55..d5758526ab1 100644 --- a/sky/engine/bindings/sky_internals.dart +++ b/sky/engine/bindings/sky_internals.dart @@ -6,8 +6,6 @@ library sky.internals; // TODO(abarth): Move these functions into dart:sky -void notifyTestComplete(String test_result) native "notifyTestComplete"; - int takeRootBundleHandle() native "takeRootBundleHandle"; int takeServiceRegistry() native "takeServiceRegistry"; int takeServicesProvidedByEmbedder() native "takeServicesProvidedByEmbedder"; diff --git a/sky/packages/sky_services/BUILD.gn b/sky/packages/sky_services/BUILD.gn index 8433d77c50b..ed8c9a10c5f 100644 --- a/sky/packages/sky_services/BUILD.gn +++ b/sky/packages/sky_services/BUILD.gn @@ -25,7 +25,6 @@ dart_pkg("sky_services") { ":copy_sky_services_license", "//sky/services/activity:interfaces", "//sky/services/media:interfaces", - "//sky/services/testing:interfaces", "//sky/services/vsync:interfaces", ] } diff --git a/sky/services/testing/BUILD.gn b/sky/services/testing/BUILD.gn deleted file mode 100644 index 880af1a90cb..00000000000 --- a/sky/services/testing/BUILD.gn +++ /dev/null @@ -1,17 +0,0 @@ -# Copyright 2014 The Chromium 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("//mojo/public/tools/bindings/mojom.gni") - -mojom("interfaces") { - sources = [ - "test_harness.mojom", - ] - - import_dirs = [ get_path_info("../../../mojo/services", "abspath") ] - - deps = [ - "//mojo/services/input_events/public/interfaces", - ] -} diff --git a/sky/services/testing/test_harness.mojom b/sky/services/testing/test_harness.mojom deleted file mode 100644 index 66bc5e62aed..00000000000 --- a/sky/services/testing/test_harness.mojom +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright 2014 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -[DartPackage="sky_services"] -module sky; - -import "mojo/services/input_events/public/interfaces/input_events.mojom"; - -interface TestHarness { - OnTestComplete(string test_result, array? pixels); - DispatchInputEvent(mojo.Event event); -}; diff --git a/sky/shell/BUILD.gn b/sky/shell/BUILD.gn index b2b36fc8363..558277748db 100644 --- a/sky/shell/BUILD.gn +++ b/sky/shell/BUILD.gn @@ -256,7 +256,6 @@ if (is_android) { deps = common_deps + [ ":common", "//mojo/common", - "//sky/services/testing:interfaces", ] } } else if (is_mac) { @@ -282,7 +281,6 @@ if (is_android) { ":common", "//mojo/common", "//sky/services/ns_net", - "//sky/services/testing:interfaces", ] } diff --git a/sky/shell/linux/main_linux.cc b/sky/shell/linux/main_linux.cc index 0521cb61303..3d6c16ff0bb 100644 --- a/sky/shell/linux/main_linux.cc +++ b/sky/shell/linux/main_linux.cc @@ -29,8 +29,11 @@ int main(int argc, const char* argv[]) { sky::shell::Shell::Init(make_scoped_ptr( new sky::shell::ServiceProviderContext(message_loop.task_runner()))); - message_loop.PostTask(FROM_HERE, base::Bind(&sky::shell::InitForTesting)); - message_loop.Run(); + if (!sky::shell::InitForTesting()) { + sky::shell::switches::PrintUsage("sky_shell"); + return 1; + } + message_loop.Run(); return 0; } diff --git a/sky/shell/linux/platform_service_provider_linux.cc b/sky/shell/linux/platform_service_provider_linux.cc index 51e0ee3280c..5cef4003121 100644 --- a/sky/shell/linux/platform_service_provider_linux.cc +++ b/sky/shell/linux/platform_service_provider_linux.cc @@ -23,10 +23,6 @@ class PlatformServiceProvider : public mojo::ServiceProvider { void ConnectToService(const mojo::String& service_name, mojo::ScopedMessagePipeHandle client_handle) override { - if (service_name == TestHarness::Name_) { - TestRunner::Shared().Create(nullptr, - mojo::MakeRequest(client_handle.Pass())); - } } private: diff --git a/sky/shell/mac/main_mac.mm b/sky/shell/mac/main_mac.mm index 428c38ed246..02814b95466 100644 --- a/sky/shell/mac/main_mac.mm +++ b/sky/shell/mac/main_mac.mm @@ -40,9 +40,9 @@ int main(int argc, const char* argv[]) { } if (command_line.HasSwitch(sky::shell::switches::kNonInteractive)) { - auto loop = base::MessageLoop::current(); - loop->PostTask(FROM_HERE, base::Bind(&sky::shell::InitForTesting)); - loop->Run(); + if (!sky::shell::InitForTesting()) + return 1; + base::MessageLoop::current()->Run(); return EXIT_SUCCESS; } diff --git a/sky/shell/mac/platform_service_provider_mac.cc b/sky/shell/mac/platform_service_provider_mac.cc index b83db2572b4..4a0b49f32b7 100644 --- a/sky/shell/mac/platform_service_provider_mac.cc +++ b/sky/shell/mac/platform_service_provider_mac.cc @@ -54,12 +54,6 @@ class PlatformServiceProvider : public mojo::ServiceProvider { vsync_.Create(nullptr, mojo::MakeRequest<::vsync::VSyncProvider>( client_handle.Pass())); } -#endif -#if !TARGET_OS_IPHONE - if (service_name == TestHarness::Name_) { - TestRunner::Shared().Create( - nullptr, mojo::MakeRequest(client_handle.Pass())); - } #endif } diff --git a/sky/shell/testing/test_runner.cc b/sky/shell/testing/test_runner.cc index 589f75f8594..241ac3c7a51 100644 --- a/sky/shell/testing/test_runner.cc +++ b/sky/shell/testing/test_runner.cc @@ -17,41 +17,6 @@ namespace sky { namespace shell { namespace { -struct UrlData { - std::string url; - std::string expected_pixel_hash; - bool enable_pixel_dumping = false; - bool is_snapshot = false; -}; - -void WaitForURL(UrlData& data) { - // A test name is formated like file:///path/to/test'--pixel-test'pixelhash - std::cin >> data.url; - - std::string pixel_switch; - std::string::size_type separator_position = data.url.find('\''); - if (separator_position != std::string::npos) { - pixel_switch = data.url.substr(separator_position + 1); - data.url.erase(separator_position); - } - - std::string pixel_hash; - separator_position = pixel_switch.find('\''); - if (separator_position != std::string::npos) { - pixel_hash = pixel_switch.substr(separator_position + 1); - pixel_switch.erase(separator_position); - } - - data.enable_pixel_dumping = pixel_switch == "--pixel-test"; - data.expected_pixel_hash = pixel_hash; -} - -void PrintAndFlush(const std::string& value) { - std::cout << value; - std::cout.flush(); -} - -const char kFileUrlPrefix[] = "file://"; static TestRunner* g_test_runner = nullptr; } // namespace @@ -77,60 +42,11 @@ TestRunner& TestRunner::Shared() { return *g_test_runner; } -void TestRunner::Start(scoped_ptr single_test) { - single_test_ = single_test.Pass(); - PrintAndFlush("#READY\n"); - ScheduleRun(); -} - -void TestRunner::OnTestComplete(const mojo::String& test_result, - const mojo::Array pixels) { - std::cout << "Content-Type: text/plain\n"; - std::cout << test_result << "\n"; - PrintAndFlush("#EOF\n"); // Text result complete - PrintAndFlush("#EOF\n"); // Pixel result complete - std::cerr << "#EOF\n"; - std::cerr.flush(); - bindings_.CloseAllBindings(); - - if (single_test_) - exit(0); - ScheduleRun(); -} - -void TestRunner::DispatchInputEvent(mojo::EventPtr event) { - // TODO(abarth): Not implemented. -} - -void TestRunner::Create(mojo::ApplicationConnection* app, - mojo::InterfaceRequest request) { - bindings_.AddBinding(this, request.Pass()); -} - -void TestRunner::ScheduleRun() { - base::MessageLoop::current()->PostTask(FROM_HERE, - base::Bind(&TestRunner::Run, weak_ptr_factory_.GetWeakPtr())); -} - -void TestRunner::Run() { - UrlData data; - if (single_test_) { - data.url = single_test_->path; - data.is_snapshot = single_test_->is_snapshot; - } else { - WaitForURL(data); - } - - std::cout << "#BEGIN\n"; - std::cout.flush(); - - if (base::StartsWithASCII(data.url, kFileUrlPrefix, true)) - base::ReplaceFirstSubstringAfterOffset(&data.url, 0, kFileUrlPrefix, ""); - - if (data.is_snapshot) - sky_engine_->RunFromSnapshot(data.url); +void TestRunner::Run(const TestDescriptor& test) { + if (test.is_snapshot) + sky_engine_->RunFromSnapshot(test.path); else - sky_engine_->RunFromFile(data.url, package_root_); + sky_engine_->RunFromFile(test.path, test.package_root); } } // namespace shell diff --git a/sky/shell/testing/test_runner.h b/sky/shell/testing/test_runner.h index 455d24505ed..1f787491881 100644 --- a/sky/shell/testing/test_runner.h +++ b/sky/shell/testing/test_runner.h @@ -12,50 +12,30 @@ #include "mojo/common/binding_set.h" #include "mojo/public/cpp/application/interface_factory_impl.h" #include "sky/services/engine/sky_engine.mojom.h" -#include "sky/services/testing/test_harness.mojom.h" namespace sky { namespace shell { class ShellView; -class TestRunner : public mojo::InterfaceFactory, - public TestHarness { +class TestRunner { public: static TestRunner& Shared(); - void set_package_root(const std::string& package_root) { - package_root_ = package_root; - } - - struct SingleTest { + struct TestDescriptor { std::string path; + std::string package_root; bool is_snapshot = false; }; - void Start(scoped_ptr single_test); - - // mojo::InterfaceFactory implementation: - void Create(mojo::ApplicationConnection* app, - mojo::InterfaceRequest request) override; + void Run(const TestDescriptor& test); private: - // TestHarness implementation: - void OnTestComplete(const mojo::String& test_result, - const mojo::Array pixels) override; - void DispatchInputEvent(mojo::EventPtr event) override; - TestRunner(); - ~TestRunner() override; - void ScheduleRun(); - void Run(); + ~TestRunner(); - std::string package_root_; scoped_ptr shell_view_; SkyEnginePtr sky_engine_; - scoped_ptr single_test_; - mojo::BindingSet bindings_; - base::WeakPtrFactory weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(TestRunner); diff --git a/sky/shell/testing/testing.cc b/sky/shell/testing/testing.cc index 493eedb4921..c40ea846be9 100644 --- a/sky/shell/testing/testing.cc +++ b/sky/shell/testing/testing.cc @@ -12,32 +12,26 @@ namespace sky { namespace shell { -void InitForTesting() { +bool InitForTesting() { base::CommandLine& command_line = *base::CommandLine::ForCurrentProcess(); blink::WebRuntimeFeatures::enableObservatory( !command_line.HasSwitch(switches::kNonInteractive)); - // Explicitly boot the shared test runner. - TestRunner& runner = TestRunner::Shared(); + TestRunner::TestDescriptor test; + test.package_root = command_line.GetSwitchValueASCII(switches::kPackageRoot); - std::string package_root = - command_line.GetSwitchValueASCII(switches::kPackageRoot); - runner.set_package_root(package_root); - - scoped_ptr single_test; if (command_line.HasSwitch(switches::kSnapshot)) { - single_test.reset(new TestRunner::SingleTest); - single_test->path = command_line.GetSwitchValueASCII(switches::kSnapshot); - single_test->is_snapshot = true; + test.path = command_line.GetSwitchValueASCII(switches::kSnapshot); + test.is_snapshot = true; } else { auto args = command_line.GetArgs(); - if (!args.empty()) { - single_test.reset(new TestRunner::SingleTest); - single_test->path = args[0]; - } + if (args.empty()) + return false; + test.path = args[0]; } - runner.Start(single_test.Pass()); + TestRunner::Shared().Run(test); + return true; } } // namespace shell diff --git a/sky/shell/testing/testing.h b/sky/shell/testing/testing.h index ca5047c2e53..0af6efd6022 100644 --- a/sky/shell/testing/testing.h +++ b/sky/shell/testing/testing.h @@ -8,7 +8,7 @@ namespace sky { namespace shell { -void InitForTesting(); +bool InitForTesting(); } // namespace shell } // namespace sky diff --git a/sky/shell/ui/internals.cc b/sky/shell/ui/internals.cc index e87b7520a05..bc6f34e1be2 100644 --- a/sky/shell/ui/internals.cc +++ b/sky/shell/ui/internals.cc @@ -26,9 +26,6 @@ Internals* GetInternals() { return static_cast(state->GetUserData(&kInternalsKey)); } -void NotifyTestComplete(Dart_NativeArguments args) { -} - void TakeRootBundleHandle(Dart_NativeArguments args) { Dart_SetIntegerReturnValue( args, GetInternals()->TakeRootBundleHandle().value()); @@ -52,7 +49,6 @@ void TakeServiceRegistry(Dart_NativeArguments args) { } const DartBuiltin::Natives kNativeFunctions[] = { - {"notifyTestComplete", NotifyTestComplete, 1}, {"takeRootBundleHandle", TakeRootBundleHandle, 0}, {"takeServiceRegistry", TakeServiceRegistry, 0}, {"takeServicesProvidedByEmbedder", TakeServicesProvidedByEmbedder, 0},