Skip to content

Commit

Permalink
Refactor ExtensionManifestTest to allow usage in src/extensions
Browse files Browse the repository at this point in the history
This refactor will allow some manifest tests running in Chrome's unit_tests
suite to move to the extensions_unittests suite.

* Rename ExtensionManifestTest to extensions::ManifestTest and move it into
  src/extensions.
* Introduce ChromeManifestTest to load manifests from Chrome's test data dir.
* Eliminate some unnecessary use of chrome::VersionInfo::Channel.
* Move SharedModuleManifestTest to extensions_unittests as an example.

FileHandlerManifestTest and ExternallyConnectableManifestTest will be next,
but require more refactoring (e.g. of permissions).

BUG=397165
TEST=unit_tests, extensions_unittests

Review URL: https://codereview.chromium.org/572813002

Cr-Commit-Position: refs/heads/master@{#295087}
  • Loading branch information
jamescook authored and Commit bot committed Sep 16, 2014
1 parent d575eaf commit bd1cc62
Show file tree
Hide file tree
Showing 70 changed files with 297 additions and 296 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include "base/strings/string_number_conversions.h"
#include "chrome/common/extensions/extension_constants.h"
#include "chrome/common/extensions/manifest_handlers/mime_types_handler.h"
#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
#include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
#include "chrome/common/extensions/manifest_url_handler.h"
#include "extensions/common/error_utils.h"
#include "extensions/common/extension_builder.h"
Expand All @@ -22,7 +22,7 @@ using extensions::ListBuilder;

namespace {

class StreamsPrivateManifestTest : public ExtensionManifestTest {
class StreamsPrivateManifestTest : public ChromeManifestTest {
};

TEST_F(StreamsPrivateManifestTest, ValidMimeTypesHandlerMIMETypes) {
Expand Down
4 changes: 2 additions & 2 deletions chrome/chrome_tests_unit.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
'../extensions/common/file_util_unittest.cc',
'../extensions/common/manifest_handlers/externally_connectable_unittest.cc',
'../extensions/common/manifest_handlers/file_handler_manifest_unittest.cc',
'../extensions/common/manifest_handlers/shared_module_manifest_unittest.cc',
# histograms.xml is analyzed by AboutFlagsHistogramTest, so this
# dependency is needed to make commit bots run unit_tests on
# histograms.xml changes.
Expand Down Expand Up @@ -1278,7 +1277,8 @@
'common/extensions/manifest_handlers/exclude_matches_manifest_unittest.cc',
'common/extensions/manifest_handlers/settings_overrides_handler_unittest.cc',
'common/extensions/manifest_handlers/ui_overrides_handler_unittest.cc',
'common/extensions/manifest_tests/extension_manifest_test.cc',
'common/extensions/manifest_tests/chrome_manifest_test.cc',
'common/extensions/manifest_tests/chrome_manifest_test.h',
'common/extensions/manifest_tests/extension_manifests_about_unittest.cc',
'common/extensions/manifest_tests/extension_manifests_background_unittest.cc',
'common/extensions/manifest_tests/extension_manifests_chromepermission_unittest.cc',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
#include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"

#include "base/command_line.h"
#include "base/strings/string_util.h"
Expand All @@ -17,7 +17,7 @@ namespace extensions {

namespace errors = manifest_errors;

class CommandsManifestTest : public ExtensionManifestTest {
class CommandsManifestTest : public ChromeManifestTest {
};

TEST_F(CommandsManifestTest, CommandManifestSimple) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// found in the LICENSE file.

#include "chrome/common/extensions/api/extension_action/action_info.h"
#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
#include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
#include "extensions/common/error_utils.h"
#include "extensions/common/extension_builder.h"
#include "extensions/common/extension_icon_set.h"
Expand All @@ -17,7 +17,7 @@ namespace errors = manifest_errors;

namespace {

class BrowserActionManifestTest : public ExtensionManifestTest {
class BrowserActionManifestTest : public ChromeManifestTest {
};

TEST_F(BrowserActionManifestTest,
Expand Down Expand Up @@ -105,7 +105,7 @@ TEST_F(BrowserActionManifestTest,

base::string16 error = ErrorUtils::FormatErrorMessageUTF16(
errors::kInvalidIconPath, "19");
LoadAndExpectError(Manifest(manifest_value.get(), "Invalid default icon"),
LoadAndExpectError(ManifestData(manifest_value.get(), "Invalid default icon"),
errors::kInvalidIconPath);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/path_service.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/extensions/api/extension_action/action_info.h"
#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
#include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
#include "extensions/common/constants.h"
#include "extensions/common/error_utils.h"
#include "extensions/common/extension.h"
Expand All @@ -15,10 +17,12 @@ namespace extensions {
namespace errors = manifest_errors;
namespace keys = manifest_keys;

class PageActionManifestTest : public ExtensionManifestTest {
class PageActionManifestTest : public ChromeManifestTest {
protected:
virtual const char* test_data_dir() OVERRIDE {
return "page_action";
virtual base::FilePath GetTestDataDir() OVERRIDE {
base::FilePath path;
PathService::Get(chrome::DIR_TEST_DATA, &path);
return path.AppendASCII("extensions").AppendASCII("page_action");
}

scoped_ptr<ActionInfo> LoadAction(const std::string& manifest_filename);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include "base/strings/string_number_conversions.h"
#include "chrome/common/extensions/api/file_browser_handlers/file_browser_handler.h"
#include "chrome/common/extensions/extension_constants.h"
#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
#include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
#include "extensions/common/error_utils.h"
#include "extensions/common/extension_builder.h"
#include "extensions/common/manifest_constants.h"
Expand All @@ -21,7 +21,7 @@ using extensions::ListBuilder;

namespace {

class FileBrowserHandlerManifestTest : public ExtensionManifestTest {
class FileBrowserHandlerManifestTest : public ChromeManifestTest {
};

TEST_F(FileBrowserHandlerManifestTest, InvalidFileBrowserHandlers) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
// found in the LICENSE file.

#include "chrome/common/extensions/api/i18n/default_locale_handler.h"
#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
#include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
#include "extensions/common/manifest_constants.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace extensions {

class DefaultLocaleManifestTest : public ExtensionManifestTest {
class DefaultLocaleManifestTest : public ChromeManifestTest {
};

TEST_F(DefaultLocaleManifestTest, DefaultLocale) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

#include "base/test/values_test_util.h"
#include "chrome/common/extensions/api/identity/oauth2_manifest_handler.h"
#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
#include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
#include "extensions/common/manifest_constants.h"
#include "testing/gtest/include/gtest/gtest.h"

Expand All @@ -25,7 +25,7 @@ const char kAutoApproveNotAllowedWarning[] =

} // namespace

class OAuth2ManifestTest : public ExtensionManifestTest {
class OAuth2ManifestTest : public ChromeManifestTest {
protected:
enum AutoApproveValue {
AUTO_APPROVE_NOT_SET,
Expand Down Expand Up @@ -107,7 +107,7 @@ TEST_F(OAuth2ManifestTest, OAuth2SectionParsing) {
ext_manifest.SetString(keys::kKey, kExtensionKey);
ext_manifest.SetBoolean(keys::kOAuth2AutoApprove, true);

Manifest manifest(&ext_manifest, "test");
ManifestData manifest(&ext_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest);
EXPECT_TRUE(extension->install_warnings().empty());
Expand All @@ -124,7 +124,7 @@ TEST_F(OAuth2ManifestTest, OAuth2SectionParsing) {
app_manifest.SetString(keys::kLaunchLocalPath, "launch.html");
app_manifest.MergeDictionary(&base_manifest);

Manifest manifest(&app_manifest, "test");
ManifestData manifest(&app_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest);
EXPECT_TRUE(extension->install_warnings().empty());
Expand All @@ -141,7 +141,7 @@ TEST_F(OAuth2ManifestTest, OAuth2SectionParsing) {
app_manifest.SetString(keys::kLaunchWebURL, "http://www.google.com");
app_manifest.MergeDictionary(&base_manifest);

Manifest manifest(&app_manifest, "test");
ManifestData manifest(&app_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest);
EXPECT_EQ(1U, extension->install_warnings().size());
Expand All @@ -159,7 +159,7 @@ TEST_F(OAuth2ManifestTest, OAuth2SectionParsing) {
TEST_F(OAuth2ManifestTest, AutoApproveNotSetExtensionNotOnWhitelist) {
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_NOT_SET, false, CLIENT_ID_DEFAULT);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest);
EXPECT_TRUE(extension->install_warnings().empty());
Expand All @@ -169,7 +169,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveNotSetExtensionNotOnWhitelist) {
TEST_F(OAuth2ManifestTest, AutoApproveFalseExtensionNotOnWhitelist) {
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_FALSE, false, CLIENT_ID_DEFAULT);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest);
EXPECT_EQ(1U, extension->install_warnings().size());
Expand All @@ -182,7 +182,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveFalseExtensionNotOnWhitelist) {
TEST_F(OAuth2ManifestTest, AutoApproveTrueExtensionNotOnWhitelist) {
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_TRUE, false, CLIENT_ID_DEFAULT);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest);
EXPECT_EQ(1U, extension->install_warnings().size());
Expand All @@ -195,7 +195,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveTrueExtensionNotOnWhitelist) {
TEST_F(OAuth2ManifestTest, AutoApproveInvalidExtensionNotOnWhitelist) {
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_INVALID, false, CLIENT_ID_DEFAULT);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest);
EXPECT_EQ(1U, extension->install_warnings().size());
Expand All @@ -208,7 +208,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveInvalidExtensionNotOnWhitelist) {
TEST_F(OAuth2ManifestTest, AutoApproveNotSetExtensionOnWhitelist) {
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_NOT_SET, true, CLIENT_ID_DEFAULT);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest);
EXPECT_TRUE(extension->install_warnings().empty());
Expand All @@ -218,7 +218,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveNotSetExtensionOnWhitelist) {
TEST_F(OAuth2ManifestTest, AutoApproveFalseExtensionOnWhitelist) {
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_FALSE, true, CLIENT_ID_DEFAULT);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest);
EXPECT_TRUE(extension->install_warnings().empty());
Expand All @@ -228,7 +228,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveFalseExtensionOnWhitelist) {
TEST_F(OAuth2ManifestTest, AutoApproveTrueExtensionOnWhitelist) {
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_TRUE, true, CLIENT_ID_DEFAULT);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest);
EXPECT_TRUE(extension->install_warnings().empty());
Expand All @@ -238,7 +238,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveTrueExtensionOnWhitelist) {
TEST_F(OAuth2ManifestTest, AutoApproveInvalidExtensionOnWhitelist) {
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_INVALID, true, CLIENT_ID_DEFAULT);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
std::string error;
scoped_refptr<extensions::Extension> extension =
LoadExtension(manifest, &error);
Expand All @@ -251,15 +251,15 @@ TEST_F(OAuth2ManifestTest, InvalidClientId) {
{
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_NOT_SET, false, CLIENT_ID_NOT_SET);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
std::string error;
LoadAndExpectError(manifest, errors::kInvalidOAuth2ClientId);
}

{
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_NOT_SET, false, CLIENT_ID_EMPTY);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
std::string error;
LoadAndExpectError(manifest, errors::kInvalidOAuth2ClientId);
}
Expand All @@ -270,7 +270,7 @@ TEST_F(OAuth2ManifestTest, ComponentInvalidClientId) {
{
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_NOT_SET, false, CLIENT_ID_NOT_SET);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
std::string error;
LoadAndExpectError(manifest,
errors::kInvalidOAuth2ClientId,
Expand All @@ -280,7 +280,7 @@ TEST_F(OAuth2ManifestTest, ComponentInvalidClientId) {
{
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_NOT_SET, false, CLIENT_ID_EMPTY);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
std::string error;
LoadAndExpectError(manifest,
errors::kInvalidOAuth2ClientId,
Expand All @@ -292,7 +292,7 @@ TEST_F(OAuth2ManifestTest, ComponentWithChromeClientId) {
{
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_TRUE, true, CLIENT_ID_NOT_SET);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest, extensions::Manifest::COMPONENT);
EXPECT_TRUE(OAuth2Info::GetOAuth2Info(extension.get()).client_id.empty());
Expand All @@ -302,7 +302,7 @@ TEST_F(OAuth2ManifestTest, ComponentWithChromeClientId) {
{
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_TRUE, true, CLIENT_ID_EMPTY);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest, extensions::Manifest::COMPONENT);
EXPECT_TRUE(OAuth2Info::GetOAuth2Info(extension.get()).client_id.empty());
Expand All @@ -313,7 +313,7 @@ TEST_F(OAuth2ManifestTest, ComponentWithChromeClientId) {
TEST_F(OAuth2ManifestTest, ComponentWithStandardClientId) {
base::DictionaryValue* ext_manifest =
CreateManifest(AUTO_APPROVE_TRUE, true, CLIENT_ID_DEFAULT);
Manifest manifest(ext_manifest, "test");
ManifestData manifest(ext_manifest, "test");
scoped_refptr<extensions::Extension> extension =
LoadAndExpectSuccess(manifest, extensions::Manifest::COMPONENT);
EXPECT_EQ("client1", OAuth2Info::GetOAuth2Info(extension.get()).client_id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
// found in the LICENSE file.

#include "base/strings/utf_string_conversions.h"
#include "chrome/common/extensions/features/feature_channel.h"
#include "chrome/common/extensions/manifest_handlers/automation.h"
#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
#include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
#include "chrome/grit/generated_resources.h"
#include "extensions/common/error_utils.h"
#include "extensions/common/manifest_constants.h"
Expand All @@ -14,7 +15,7 @@

namespace extensions {

class AutomationManifestTest : public ExtensionManifestTest {
class AutomationManifestTest : public ChromeManifestTest {
public:
AutomationManifestTest() : channel_(chrome::VersionInfo::CHANNEL_UNKNOWN) {}

Expand Down Expand Up @@ -156,7 +157,7 @@ TEST_F(AutomationManifestTest, EmptyMatches) {
TEST_F(AutomationManifestTest, NoValidMatches) {
std::string error;
scoped_refptr<Extension> extension =
LoadExtension(Manifest("automation_no_valid_matches.json"), &error);
LoadExtension(ManifestData("automation_no_valid_matches.json"), &error);
ASSERT_TRUE(extension.get());
EXPECT_EQ("", error);
EXPECT_EQ(2u, extension->install_warnings().size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include "base/command_line.h"
#include "base/strings/string_number_conversions.h"
#include "chrome/common/extensions/manifest_handlers/content_scripts_handler.h"
#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
#include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
#include "extensions/common/error_utils.h"
#include "extensions/common/extension.h"
#include "extensions/common/manifest_constants.h"
Expand All @@ -16,7 +16,7 @@ namespace extensions {

namespace errors = manifest_errors;

class ContentScriptsManifestTest : public ExtensionManifestTest {
class ContentScriptsManifestTest : public ChromeManifestTest {
};

TEST_F(ContentScriptsManifestTest, MatchPattern) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.


#include "chrome/common/extensions/manifest_tests/extension_manifest_test.h"
#include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
#include "extensions/common/extension.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace extensions {

class ExcludeMatchesManifestTest : public ExtensionManifestTest {
class ExcludeMatchesManifestTest : public ChromeManifestTest {
};

TEST_F(ExcludeMatchesManifestTest, ExcludeMatchPatterns) {
Expand Down
Loading

0 comments on commit bd1cc62

Please sign in to comment.