From 5c855a28156ed8f4d3f191ea7a1f9ee3dc777ee7 Mon Sep 17 00:00:00 2001 From: "thiago.santos" Date: Mon, 22 Dec 2014 18:00:47 -0800 Subject: [PATCH] Move system.display tests to extensions/ Also copy a few helper functions from extension_function_test_utils.h to api_test_utils.h so we don't need to depend on chrome/. These helper functions are needed by tests other than the display test and will help with the migration. BUG=392842 Review URL: https://codereview.chromium.org/779083002 Cr-Commit-Position: refs/heads/master@{#309514} --- .../extensions/extension_function_test_utils.cc | 43 ++---- chrome/chrome_tests.gypi | 1 - .../api/system_display/system_display_apitest.cc | 169 ++++++++------------- extensions/browser/api_test_utils.cc | 52 +++++++ extensions/browser/api_test_utils.h | 27 +++- extensions/shell/app_shell.gyp | 1 + .../test/data}/system/display/manifest.json | 0 .../test/data}/system/display/test_display_api.js | 2 +- 8 files changed, 151 insertions(+), 144 deletions(-) rename {chrome/browser/extensions => extensions/browser}/api/system_display/system_display_apitest.cc (58%) rename {chrome/test/data/extensions/api_test => extensions/test/data}/system/display/manifest.json (100%) rename {chrome/test/data/extensions/api_test => extensions/test/data}/system/display/test_display_api.js (97%) diff --git a/chrome/browser/extensions/extension_function_test_utils.cc b/chrome/browser/extensions/extension_function_test_utils.cc index 7cdb98dc37ad..e872a1c7cfba 100644 --- a/chrome/browser/extensions/extension_function_test_utils.cc +++ b/chrome/browser/extensions/extension_function_test_utils.cc @@ -60,32 +60,20 @@ base::ListValue* ParseList(const std::string& data) { base::DictionaryValue* ParseDictionary( const std::string& data) { - base::Value* result = ParseJSON(data); - base::DictionaryValue* dict = NULL; - result->GetAsDictionary(&dict); - return dict; + return extensions::api_test_utils::ParseDictionary(data); } bool GetBoolean(const base::DictionaryValue* val, const std::string& key) { - bool result = false; - if (!val->GetBoolean(key, &result)) - ADD_FAILURE() << key << " does not exist or is not a boolean."; - return result; + return extensions::api_test_utils::GetBoolean(val, key); } int GetInteger(const base::DictionaryValue* val, const std::string& key) { - int result = 0; - if (!val->GetInteger(key, &result)) - ADD_FAILURE() << key << " does not exist or is not an integer."; - return result; + return extensions::api_test_utils::GetInteger(val, key); } std::string GetString(const base::DictionaryValue* val, const std::string& key) { - std::string result; - if (!val->GetString(key, &result)) - ADD_FAILURE() << key << " does not exist or is not a string."; - return result; + return extensions::api_test_utils::GetString(val, key); } base::DictionaryValue* ToDictionary(base::Value* val) { @@ -104,33 +92,22 @@ scoped_refptr CreateEmptyExtensionWithLocation( Manifest::Location location) { scoped_ptr test_extension_value( ParseDictionary("{\"name\": \"Test\", \"version\": \"1.0\"}")); - return CreateExtension(location, test_extension_value.get(), std::string()); + return extensions::api_test_utils::CreateExtension( + location, test_extension_value.get(), std::string()); } scoped_refptr CreateExtension( base::DictionaryValue* test_extension_value) { - return CreateExtension(Manifest::INTERNAL, test_extension_value, - std::string()); + return extensions::api_test_utils::CreateExtension( + Manifest::INTERNAL, test_extension_value, std::string()); } scoped_refptr CreateExtension( Manifest::Location location, base::DictionaryValue* test_extension_value, const std::string& id_input) { - std::string error; - const base::FilePath test_extension_path; - std::string id; - if (!id_input.empty()) - id = crx_file::id_util::GenerateId(id_input); - scoped_refptr extension(Extension::Create( - test_extension_path, - location, - *test_extension_value, - Extension::NO_FLAGS, - id, - &error)); - EXPECT_TRUE(error.empty()) << "Could not parse test extension " << error; - return extension; + return extensions::api_test_utils::CreateExtension( + location, test_extension_value, id_input); } bool HasPrivacySensitiveFields(base::DictionaryValue* val) { diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index a6e6f669fdb2..eebc53f9dc5f 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -335,7 +335,6 @@ 'browser/extensions/api/sync_file_system/sync_file_system_browsertest.cc', 'browser/extensions/api/sync_file_system/sync_file_system_apitest.cc', 'browser/extensions/api/system_indicator/system_indicator_apitest.cc', - 'browser/extensions/api/system_display/system_display_apitest.cc', 'browser/extensions/api/system_private/system_private_apitest.cc', 'browser/extensions/api/tab_capture/tab_capture_apitest.cc', 'browser/extensions/api/tabs/tabs_test.cc', diff --git a/chrome/browser/extensions/api/system_display/system_display_apitest.cc b/extensions/browser/api/system_display/system_display_apitest.cc similarity index 58% rename from chrome/browser/extensions/api/system_display/system_display_apitest.cc rename to extensions/browser/api/system_display/system_display_apitest.cc index c0418f47ff08..ee7f0f626875 100644 --- a/chrome/browser/extensions/api/system_display/system_display_apitest.cc +++ b/extensions/browser/api/system_display/system_display_apitest.cc @@ -4,60 +4,21 @@ #include "base/debug/leak_annotations.h" #include "base/strings/string_number_conversions.h" -#include "chrome/browser/extensions/extension_apitest.h" -#include "chrome/browser/extensions/extension_function_test_utils.h" #include "extensions/browser/api/system_display/display_info_provider.h" #include "extensions/browser/api/system_display/system_display_api.h" +#include "extensions/browser/api_test_utils.h" #include "extensions/common/api/system_display.h" +#include "extensions/shell/test/shell_apitest.h" #include "ui/gfx/display.h" #include "ui/gfx/display_observer.h" #include "ui/gfx/screen.h" -#if defined(OS_CHROMEOS) -#include "ash/display/screen_ash.h" -#include "ash/shell.h" -#endif - -namespace utils = extension_function_test_utils; - namespace extensions { using core_api::system_display::Bounds; using core_api::system_display::DisplayUnitInfo; using gfx::Screen; -#if defined(OS_CHROMEOS) -class MockScreen : public ash::ScreenAsh { - public: - MockScreen() { - for (int i = 0; i < 4; i++) { - gfx::Rect bounds(0, 0, 1280, 720); - gfx::Rect work_area(0, 0, 960, 720); - gfx::Display display(i, bounds); - display.set_work_area(work_area); - displays_.push_back(display); - } - } - virtual ~MockScreen() {} - - protected: - // Overridden from gfx::Screen: - virtual int GetNumDisplays() const override { - return displays_.size(); - } - virtual std::vector GetAllDisplays() const override { - return displays_; - } - virtual gfx::Display GetPrimaryDisplay() const override { - return displays_[0]; - } - - private: - std::vector displays_; - - DISALLOW_COPY_AND_ASSIGN(MockScreen); -}; -#else class MockScreen : public Screen { public: MockScreen() { @@ -80,7 +41,9 @@ class MockScreen : public Screen { gfx::NativeWindow GetWindowAtScreenPoint(const gfx::Point& point) override { return gfx::NativeWindow(); } - int GetNumDisplays() const override { return displays_.size(); } + int GetNumDisplays() const override { + return static_cast(displays_.size()); + } std::vector GetAllDisplays() const override { return displays_; } @@ -102,7 +65,6 @@ class MockScreen : public Screen { DISALLOW_COPY_AND_ASSIGN(MockScreen); }; -#endif class MockDisplayInfoProvider : public DisplayInfoProvider { public: @@ -126,9 +88,7 @@ class MockDisplayInfoProvider : public DisplayInfoProvider { return set_info_value_.Pass(); } - std::string GetSetInfoDisplayId() const { - return set_info_display_id_; - } + std::string GetSetInfoDisplayId() const { return set_info_display_id_; } private: // Update the content of the |unit| obtained for |display| using @@ -160,29 +120,21 @@ class MockDisplayInfoProvider : public DisplayInfoProvider { DISALLOW_COPY_AND_ASSIGN(MockDisplayInfoProvider); }; -class SystemDisplayApiTest: public ExtensionApiTest { +class SystemDisplayApiTest : public ShellApiTest { public: - SystemDisplayApiTest() : provider_(new MockDisplayInfoProvider), - screen_(new MockScreen) {} + SystemDisplayApiTest() + : provider_(new MockDisplayInfoProvider), screen_(new MockScreen) {} ~SystemDisplayApiTest() override {} void SetUpOnMainThread() override { - ExtensionApiTest::SetUpOnMainThread(); + ShellApiTest::SetUpOnMainThread(); ANNOTATE_LEAKING_OBJECT_PTR( gfx::Screen::GetScreenByType(gfx::SCREEN_TYPE_NATIVE)); gfx::Screen::SetScreenInstance(gfx::SCREEN_TYPE_NATIVE, screen_.get()); DisplayInfoProvider::InitializeForTesting(provider_.get()); } - void TearDownOnMainThread() override { -#if defined(OS_CHROMEOS) - gfx::Screen::SetScreenInstance(gfx::SCREEN_TYPE_NATIVE, - ash::Shell::GetScreen()); -#endif - ExtensionApiTest::TearDownOnMainThread(); - } - protected: scoped_ptr provider_; scoped_ptr screen_; @@ -192,20 +144,20 @@ class SystemDisplayApiTest: public ExtensionApiTest { }; IN_PROC_BROWSER_TEST_F(SystemDisplayApiTest, GetDisplay) { - ASSERT_TRUE(RunPlatformAppTest("system/display")) << message_; + ASSERT_TRUE(RunAppTest("system/display")) << message_; } #if !defined(OS_CHROMEOS) IN_PROC_BROWSER_TEST_F(SystemDisplayApiTest, SetDisplay) { - scoped_refptr - set_info_function(new SystemDisplaySetDisplayPropertiesFunction()); + scoped_refptr set_info_function( + new SystemDisplaySetDisplayPropertiesFunction()); set_info_function->set_has_callback(true); - EXPECT_EQ("Function available only on ChromeOS.", - utils::RunFunctionAndReturnError(set_info_function.get(), - "[\"display_id\", {}]", - browser())); + EXPECT_EQ( + "Function available only on ChromeOS.", + api_test_utils::RunFunctionAndReturnError( + set_info_function.get(), "[\"display_id\", {}]", browser_context())); scoped_ptr set_info = provider_->GetSetInfoValue(); EXPECT_FALSE(set_info); @@ -214,56 +166,58 @@ IN_PROC_BROWSER_TEST_F(SystemDisplayApiTest, SetDisplay) { #if defined(OS_CHROMEOS) IN_PROC_BROWSER_TEST_F(SystemDisplayApiTest, SetDisplayNotKioskEnabled) { - scoped_ptr test_extension_value(utils::ParseDictionary( - "{\n" - " \"name\": \"Test\",\n" - " \"version\": \"1.0\",\n" - " \"app\": {\n" - " \"background\": {\n" - " \"scripts\": [\"background.js\"]\n" - " }\n" - " }\n" - "}")); + scoped_ptr test_extension_value( + api_test_utils::ParseDictionary( + "{\n" + " \"name\": \"Test\",\n" + " \"version\": \"1.0\",\n" + " \"app\": {\n" + " \"background\": {\n" + " \"scripts\": [\"background.js\"]\n" + " }\n" + " }\n" + "}")); scoped_refptr test_extension( - utils::CreateExtension(test_extension_value.get())); + api_test_utils::CreateExtension(test_extension_value.get())); - scoped_refptr - set_info_function(new SystemDisplaySetDisplayPropertiesFunction()); + scoped_refptr set_info_function( + new SystemDisplaySetDisplayPropertiesFunction()); set_info_function->set_extension(test_extension.get()); set_info_function->set_has_callback(true); - EXPECT_EQ("The extension needs to be kiosk enabled to use the function.", - utils::RunFunctionAndReturnError(set_info_function.get(), - "[\"display_id\", {}]", - browser())); + EXPECT_EQ( + "The extension needs to be kiosk enabled to use the function.", + api_test_utils::RunFunctionAndReturnError( + set_info_function.get(), "[\"display_id\", {}]", browser_context())); scoped_ptr set_info = provider_->GetSetInfoValue(); EXPECT_FALSE(set_info); } IN_PROC_BROWSER_TEST_F(SystemDisplayApiTest, SetDisplayKioskEnabled) { - scoped_ptr test_extension_value(utils::ParseDictionary( - "{\n" - " \"name\": \"Test\",\n" - " \"version\": \"1.0\",\n" - " \"app\": {\n" - " \"background\": {\n" - " \"scripts\": [\"background.js\"]\n" - " }\n" - " },\n" - " \"kiosk_enabled\": true\n" - "}")); + scoped_ptr test_extension_value( + api_test_utils::ParseDictionary( + "{\n" + " \"name\": \"Test\",\n" + " \"version\": \"1.0\",\n" + " \"app\": {\n" + " \"background\": {\n" + " \"scripts\": [\"background.js\"]\n" + " }\n" + " },\n" + " \"kiosk_enabled\": true\n" + "}")); scoped_refptr test_extension( - utils::CreateExtension(test_extension_value.get())); + api_test_utils::CreateExtension(test_extension_value.get())); - scoped_refptr - set_info_function(new SystemDisplaySetDisplayPropertiesFunction()); + scoped_refptr set_info_function( + new SystemDisplaySetDisplayPropertiesFunction()); set_info_function->set_has_callback(true); set_info_function->set_extension(test_extension.get()); - ASSERT_TRUE(utils::RunFunction( + ASSERT_TRUE(api_test_utils::RunFunction( set_info_function.get(), "[\"display_id\", {\n" " \"isPrimary\": true,\n" @@ -273,23 +227,22 @@ IN_PROC_BROWSER_TEST_F(SystemDisplayApiTest, SetDisplayKioskEnabled) { " \"rotation\": 90,\n" " \"overscan\": {\"left\": 1, \"top\": 2, \"right\": 3, \"bottom\": 4}\n" "}]", - browser(), - utils::NONE)); + browser_context())); scoped_ptr set_info = provider_->GetSetInfoValue(); ASSERT_TRUE(set_info); - EXPECT_TRUE(utils::GetBoolean(set_info.get(), "isPrimary")); + EXPECT_TRUE(api_test_utils::GetBoolean(set_info.get(), "isPrimary")); EXPECT_EQ("mirroringId", - utils::GetString(set_info.get(), "mirroringSourceId")); - EXPECT_EQ(100, utils::GetInteger(set_info.get(), "boundsOriginX")); - EXPECT_EQ(200, utils::GetInteger(set_info.get(), "boundsOriginY")); - EXPECT_EQ(90, utils::GetInteger(set_info.get(), "rotation")); + api_test_utils::GetString(set_info.get(), "mirroringSourceId")); + EXPECT_EQ(100, api_test_utils::GetInteger(set_info.get(), "boundsOriginX")); + EXPECT_EQ(200, api_test_utils::GetInteger(set_info.get(), "boundsOriginY")); + EXPECT_EQ(90, api_test_utils::GetInteger(set_info.get(), "rotation")); base::DictionaryValue* overscan; ASSERT_TRUE(set_info->GetDictionary("overscan", &overscan)); - EXPECT_EQ(1, utils::GetInteger(overscan, "left")); - EXPECT_EQ(2, utils::GetInteger(overscan, "top")); - EXPECT_EQ(3, utils::GetInteger(overscan, "right")); - EXPECT_EQ(4, utils::GetInteger(overscan, "bottom")); + EXPECT_EQ(1, api_test_utils::GetInteger(overscan, "left")); + EXPECT_EQ(2, api_test_utils::GetInteger(overscan, "top")); + EXPECT_EQ(3, api_test_utils::GetInteger(overscan, "right")); + EXPECT_EQ(4, api_test_utils::GetInteger(overscan, "bottom")); EXPECT_EQ("display_id", provider_->GetSetInfoDisplayId()); } diff --git a/extensions/browser/api_test_utils.cc b/extensions/browser/api_test_utils.cc index 23c0efeaa183..8d215a5fc29d 100644 --- a/extensions/browser/api_test_utils.cc +++ b/extensions/browser/api_test_utils.cc @@ -7,6 +7,7 @@ #include "base/json/json_reader.h" #include "base/memory/scoped_ptr.h" #include "base/values.h" +#include "components/crx_file/id_util.h" #include "content/public/browser/browser_context.h" #include "content/public/test/test_utils.h" #include "extensions/browser/extension_function.h" @@ -83,6 +84,57 @@ namespace extensions { namespace api_test_utils { +base::DictionaryValue* ParseDictionary(const std::string& data) { + base::Value* result = ParseJSON(data); + base::DictionaryValue* dict = NULL; + result->GetAsDictionary(&dict); + return dict; +} + +bool GetBoolean(const base::DictionaryValue* val, const std::string& key) { + bool result = false; + if (!val->GetBoolean(key, &result)) + ADD_FAILURE() << key << " does not exist or is not a boolean."; + return result; +} + +int GetInteger(const base::DictionaryValue* val, const std::string& key) { + int result = 0; + if (!val->GetInteger(key, &result)) + ADD_FAILURE() << key << " does not exist or is not an integer."; + return result; +} + +std::string GetString(const base::DictionaryValue* val, + const std::string& key) { + std::string result; + if (!val->GetString(key, &result)) + ADD_FAILURE() << key << " does not exist or is not a string."; + return result; +} + +scoped_refptr CreateExtension( + Manifest::Location location, + base::DictionaryValue* test_extension_value, + const std::string& id_input) { + std::string error; + const base::FilePath test_extension_path; + std::string id; + if (!id_input.empty()) + id = crx_file::id_util::GenerateId(id_input); + scoped_refptr extension( + Extension::Create(test_extension_path, location, *test_extension_value, + Extension::NO_FLAGS, id, &error)); + EXPECT_TRUE(error.empty()) << "Could not parse test extension " << error; + return extension; +} + +scoped_refptr CreateExtension( + base::DictionaryValue* test_extension_value) { + return CreateExtension(Manifest::INTERNAL, test_extension_value, + std::string()); +} + base::Value* RunFunctionWithDelegateAndReturnSingleResult( UIThreadExtensionFunction* function, const std::string& args, diff --git a/extensions/browser/api_test_utils.h b/extensions/browser/api_test_utils.h index 3a059a65aafb..9b6eeb5637d1 100644 --- a/extensions/browser/api_test_utils.h +++ b/extensions/browser/api_test_utils.h @@ -7,11 +7,14 @@ #include +#include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" +#include "extensions/common/manifest.h" class UIThreadExtensionFunction; namespace base { +class DictionaryValue; class ListValue; class Value; } @@ -21,6 +24,7 @@ class BrowserContext; } namespace extensions { +class Extension; class ExtensionFunctionDispatcher; // TODO(yoz): crbug.com/394840: Remove duplicate functionality in @@ -32,6 +36,27 @@ namespace api_test_utils { enum RunFunctionFlags { NONE = 0, INCLUDE_INCOGNITO = 1 << 0 }; +// Parse JSON and return as the specified type, or NULL if the JSON is invalid +// or not the specified type. +base::DictionaryValue* ParseDictionary(const std::string& data); + +// Get |key| from |val| as the specified type. If |key| does not exist, or is +// not of the specified type, adds a failure to the current test and returns +// false, 0, empty string, etc. +bool GetBoolean(const base::DictionaryValue* val, const std::string& key); +int GetInteger(const base::DictionaryValue* val, const std::string& key); +std::string GetString(const base::DictionaryValue* val, const std::string& key); + +// Creates an extension instance with a specified extension value that can be +// attached to an ExtensionFunction before running. +scoped_refptr CreateExtension( + base::DictionaryValue* test_extension_value); + +scoped_refptr CreateExtension( + extensions::Manifest::Location location, + base::DictionaryValue* test_extension_value, + const std::string& id_input); + // Run |function| with |args| and return the result. Adds an error to the // current test if |function| returns an error. Takes ownership of // |function|. The caller takes ownership of the result. @@ -94,7 +119,7 @@ bool RunFunction(UIThreadExtensionFunction* function, scoped_ptr dispatcher, RunFunctionFlags flags); -} // namespace function_test_utils +} // namespace api_test_utils } // namespace extensions #endif // EXTENSIONS_BROWSER_API_TEST_UTILS_H_ diff --git a/extensions/shell/app_shell.gyp b/extensions/shell/app_shell.gyp index 4fe387647099..69581f2c6417 100644 --- a/extensions/shell/app_shell.gyp +++ b/extensions/shell/app_shell.gyp @@ -315,6 +315,7 @@ '../browser/api/sockets_tcp/sockets_tcp_apitest.cc', '../browser/api/sockets_udp/sockets_udp_apitest.cc', '../browser/api/system_cpu/system_cpu_apitest.cc', + '../browser/api/system_display/system_display_apitest.cc', '../browser/api/system_memory/system_memory_apitest.cc', '../browser/api/system_network/system_network_apitest.cc', '../browser/api/system_storage/storage_api_test_util.cc', diff --git a/chrome/test/data/extensions/api_test/system/display/manifest.json b/extensions/test/data/system/display/manifest.json similarity index 100% rename from chrome/test/data/extensions/api_test/system/display/manifest.json rename to extensions/test/data/system/display/manifest.json diff --git a/chrome/test/data/extensions/api_test/system/display/test_display_api.js b/extensions/test/data/system/display/test_display_api.js similarity index 97% rename from chrome/test/data/extensions/api_test/system/display/test_display_api.js rename to extensions/test/data/system/display/test_display_api.js index 0c7b53bc84ac..c55c2c1a672f 100644 --- a/chrome/test/data/extensions/api_test/system/display/test_display_api.js +++ b/extensions/test/data/system/display/test_display_api.js @@ -3,7 +3,7 @@ // found in the LICENSE file. // system.display api test -// browser_tests --gtest_filter=SystemDisplayApiTest.* +// app_shell_browsertests --gtest_filter=SystemDisplayApiTest.* chrome.test.runTests([ function testGet() { -- 2.11.4.GIT