Skip to content

Commit

Permalink
[SH tests] Move DataDrivenTest to reusable location
Browse files Browse the repository at this point in the history
DataDrivenTest was initially created for autofill feature but it is
generic enough that can be reused by different features. In order to
reuse it for shared highlighting, move it out of autofill/ dir to
testing/ dir.

Bug: 1263185
Change-Id: I2adb40d880fc6eac6affe39601237b9068378bc0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3242864
Commit-Queue: Gayane Petrosyan <gayane@google.com>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: sebsg <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#938382}
  • Loading branch information
Gayane Petrosyan authored and Chromium LUCI CQ committed Nov 4, 2021
1 parent 1c54d1a commit 945148b
Show file tree
Hide file tree
Showing 10 changed files with 86 additions and 68 deletions.
14 changes: 7 additions & 7 deletions chrome/browser/autofill/form_structure_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "components/autofill/content/browser/content_autofill_driver_factory.h"
#include "components/autofill/core/browser/autofill_experiments.h"
#include "components/autofill/core/browser/browser_autofill_manager.h"
#include "components/autofill/core/browser/data_driven_test.h"
#include "components/autofill/core/browser/form_structure.h"
#include "components/autofill/core/browser/pattern_provider/pattern_configuration_parser.h"
#include "components/autofill/core/common/autofill_features.h"
Expand All @@ -40,6 +39,7 @@
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h"
#include "testing/data_driven_testing/data_driven_test.h"
#include "url/gurl.h"

#if defined(OS_MAC)
Expand All @@ -53,6 +53,7 @@ using net::test_server::BasicHttpResponse;
using net::test_server::HttpRequest;
using net::test_server::HttpResponse;

const base::FilePath::CharType kFeatureName[] = FILE_PATH_LITERAL("autofill");
const base::FilePath::CharType kTestName[] = FILE_PATH_LITERAL("heuristics");

// To disable a data driven test, please add the name of the test file
Expand All @@ -75,7 +76,7 @@ const base::FilePath& GetTestDataDir() {

const base::FilePath GetInputDir() {
static base::FilePath input_dir = GetTestDataDir()
.AppendASCII("autofill")
.Append(kFeatureName)
.Append(kTestName)
.AppendASCII("input");
return input_dir;
Expand Down Expand Up @@ -145,8 +146,8 @@ std::string FormStructuresToString(
// heuristically detected type for each field.
class FormStructureBrowserTest
: public InProcessBrowserTest,
public DataDrivenTest,
public ::testing::WithParamInterface<base::FilePath> {
public testing::DataDrivenTest,
public testing::WithParamInterface<base::FilePath> {
public:
FormStructureBrowserTest(const FormStructureBrowserTest&) = delete;
FormStructureBrowserTest& operator=(const FormStructureBrowserTest&) = delete;
Expand Down Expand Up @@ -177,7 +178,7 @@ class FormStructureBrowserTest
};

FormStructureBrowserTest::FormStructureBrowserTest()
: DataDrivenTest(GetTestDataDir()) {
: ::testing::DataDrivenTest(GetTestDataDir(), kFeatureName, kTestName) {
feature_list_.InitWithFeatures(
// Enabled
{// TODO(crbug.com/1187842): Remove once experiment is over.
Expand Down Expand Up @@ -267,8 +268,7 @@ IN_PROC_BROWSER_TEST_P(FormStructureBrowserTest, DataDrivenHeuristics) {
LOG(INFO) << GetParam().MaybeAsASCII();
bool is_expected_to_pass =
!base::Contains(GetFailingTestNames(), GetParam().BaseName().value());
RunOneDataDrivenTest(GetParam(), GetOutputDirectory(kTestName),
is_expected_to_pass);
RunOneDataDrivenTest(GetParam(), GetOutputDirectory(), is_expected_to_pass);
}

INSTANTIATE_TEST_SUITE_P(AllForms,
Expand Down
1 change: 1 addition & 0 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1322,6 +1322,7 @@ if (!is_android && !is_fuchsia) {
"//services/strings",
"//skia",
"//storage/browser:test_support",
"//testing/data_driven_testing",
"//testing/gmock",
"//testing/gtest",
"//testing/perf",
Expand Down
3 changes: 1 addition & 2 deletions components/autofill/core/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -539,8 +539,6 @@ static_library("test_support") {
"autofill_form_test_utils.h",
"autofill_test_utils.cc",
"autofill_test_utils.h",
"data_driven_test.cc",
"data_driven_test.h",
"geo/alternative_state_name_map_test_utils.cc",
"geo/alternative_state_name_map_test_utils.h",
"geo/test_region_data_loader.cc",
Expand Down Expand Up @@ -890,6 +888,7 @@ source_set("unit_tests") {
"//services/network:test_support",
"//services/network/public/cpp",
"//sql",
"//testing/data_driven_testing",
"//testing/gmock",
"//testing/gtest",
"//third_party/libaddressinput:test_support",
Expand Down
35 changes: 15 additions & 20 deletions components/autofill/core/browser/autofill_merge_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include "build/build_config.h"
#include "components/autofill/core/browser/autofill_test_utils.h"
#include "components/autofill/core/browser/autofill_type.h"
#include "components/autofill/core/browser/data_driven_test.h"
#include "components/autofill/core/browser/data_model/autofill_profile_comparator.h"
#include "components/autofill/core/browser/form_data_importer.h"
#include "components/autofill/core/browser/form_structure.h"
Expand All @@ -30,6 +29,7 @@
#include "components/autofill/core/browser/test_autofill_client.h"
#include "components/autofill/core/common/autofill_features.h"
#include "components/autofill/core/common/form_data.h"
#include "testing/data_driven_testing/data_driven_test.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"

Expand All @@ -40,7 +40,7 @@
namespace autofill {

namespace {

const base::FilePath::CharType kFeatureName[] = FILE_PATH_LITERAL("autofill");
const base::FilePath::CharType kTestName[] = FILE_PATH_LITERAL("merge");
const base::FilePath::CharType kTestNameStructuredNames[] =
FILE_PATH_LITERAL("merge_structured_names");
Expand Down Expand Up @@ -73,13 +73,17 @@ const base::FilePath& GetTestDataDir() {
return *dir;
}

const std::vector<base::FilePath> GetTestFiles() {
base::FilePath dir = GetTestDataDir();
const base::FilePath::StringType GetTestName() {
// TODO(crbug.com/1103421): Clean legacy implementation once structured names
// are fully launched.
bool structured_names = base::FeatureList::IsEnabled(
features::kAutofillEnableSupportForMoreStructureInNames);
dir = dir.AppendASCII("autofill")
.AppendASCII(structured_names ? "merge_structured_names" : "merge")
.AppendASCII("input");
return structured_names ? kTestNameStructuredNames : kTestName;
}
const std::vector<base::FilePath> GetTestFiles() {
base::FilePath dir = GetTestDataDir();

dir = dir.AppendASCII("autofill").Append(GetTestName()).AppendASCII("input");
base::FileEnumerator input_files(dir, false, base::FileEnumerator::FILES,
kFileNamePattern);
std::vector<base::FilePath> files;
Expand Down Expand Up @@ -174,7 +178,7 @@ std::vector<AutofillProfile*> PersonalDataManagerMock::GetProfiles() const {
// corresponding output file is a dump of the saved profiles that result from
// importing the input profiles. The output file format is identical to the
// input format.
class AutofillMergeTest : public DataDrivenTest,
class AutofillMergeTest : public testing::DataDrivenTest,
public testing::TestWithParam<base::FilePath> {
public:
AutofillMergeTest(const AutofillMergeTest&) = delete;
Expand Down Expand Up @@ -208,7 +212,8 @@ class AutofillMergeTest : public DataDrivenTest,
std::map<std::string, ServerFieldType> string_to_field_type_map_;
};

AutofillMergeTest::AutofillMergeTest() : DataDrivenTest(GetTestDataDir()) {
AutofillMergeTest::AutofillMergeTest()
: testing::DataDrivenTest(GetTestDataDir(), kFeatureName, GetTestName()) {
CountryNames::SetLocaleString("en-US");
for (size_t i = NO_SERVER_DATA; i < MAX_VALID_FIELD_TYPE; ++i) {
ServerFieldType field_type = ToSafeServerFieldType(i, MAX_VALID_FIELD_TYPE);
Expand Down Expand Up @@ -322,17 +327,7 @@ ServerFieldType AutofillMergeTest::StringToFieldType(const std::string& str) {

TEST_P(AutofillMergeTest, DataDrivenMergeProfiles) {
const bool kIsExpectedToPass = true;
// TODO(crbug.com/1103421): Clean legacy implementation once structured names
// are fully launched.
if (base::FeatureList::IsEnabled(
features::kAutofillEnableSupportForMoreStructureInNames)) {
RunOneDataDrivenTest(GetParam(),
GetOutputDirectory(kTestNameStructuredNames),
kIsExpectedToPass);
} else {
RunOneDataDrivenTest(GetParam(), GetOutputDirectory(kTestName),
kIsExpectedToPass);
}
RunOneDataDrivenTest(GetParam(), GetOutputDirectory(), kIsExpectedToPass);
}

INSTANTIATE_TEST_SUITE_P(All,
Expand Down
1 change: 1 addition & 0 deletions ios/chrome/browser/autofill/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,7 @@ source_set("unit_tests") {
"//ios/testing:block_swizzler",
"//ios/web/public/js_messaging",
"//ios/web/public/test",
"//testing/data_driven_testing",
"//testing/gtest",
"//third_party/leveldatabase",
"//third_party/ocmock",
Expand Down
13 changes: 7 additions & 6 deletions ios/chrome/browser/autofill/form_structure_browsertest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#import "base/test/ios/wait_util.h"
#include "base/test/scoped_feature_list.h"
#include "components/autofill/core/browser/browser_autofill_manager.h"
#include "components/autofill/core/browser/data_driven_test.h"
#include "components/autofill/core/browser/form_structure.h"
#include "components/autofill/core/common/autofill_features.h"
#include "components/autofill/core/common/autofill_payments_features.h"
Expand All @@ -38,6 +37,7 @@
#include "ios/web/public/js_messaging/web_frame.h"
#import "ios/web/public/js_messaging/web_frames_manager.h"
#import "ios/web/public/web_state.h"
#include "testing/data_driven_testing/data_driven_test.h"

#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
Expand All @@ -50,6 +50,7 @@

namespace {

const base::FilePath::CharType kFeatureName[] = FILE_PATH_LITERAL("autofill");
const base::FilePath::CharType kTestName[] = FILE_PATH_LITERAL("heuristics");

base::FilePath GetTestDataDir() {
Expand All @@ -65,7 +66,7 @@
return dir.AppendASCII("components")
.AppendASCII("test")
.AppendASCII("data")
.AppendASCII("autofill")
.Append(kFeatureName)
.Append(kTestName)
.AppendASCII("input");
}
Expand All @@ -77,7 +78,7 @@
return dir.AppendASCII("components")
.AppendASCII("test")
.AppendASCII("data")
.AppendASCII("autofill")
.Append(kFeatureName)
.Append(kTestName)
.AppendASCII("output");
}
Expand Down Expand Up @@ -107,8 +108,8 @@
// TODO(crbug.com/245246): Unify the tests.
class FormStructureBrowserTest
: public ChromeWebTest,
public DataDrivenTest,
public ::testing::WithParamInterface<base::FilePath> {
public testing::DataDrivenTest,
public testing::WithParamInterface<base::FilePath> {
public:
FormStructureBrowserTest(const FormStructureBrowserTest&) = delete;
FormStructureBrowserTest& operator=(const FormStructureBrowserTest&) = delete;
Expand Down Expand Up @@ -140,7 +141,7 @@

FormStructureBrowserTest::FormStructureBrowserTest()
: ChromeWebTest(std::make_unique<ChromeWebClient>()),
DataDrivenTest(GetTestDataDir()) {
DataDrivenTest(GetTestDataDir(), kFeatureName, kTestName) {
feature_list_.InitWithFeatures(
// Enabled
{// TODO(crbug.com/1098943): Remove once experiment is over.
Expand Down
18 changes: 18 additions & 0 deletions testing/data_driven_testing/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Copyright 2021 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.

static_library("data_driven_testing") {
testonly = true

sources = [
"data_driven_test.cc",
"data_driven_test.h",
]

deps = [
"//base",
"//testing/gtest",
"//third_party/re2",
]
}
2 changes: 2 additions & 0 deletions testing/data_driven_testing/OWNERS
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
file://components/autofill/OWNERS
file://components/shared_highlighting/OWNERS
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 "components/autofill/core/browser/data_driven_test.h"
#include "testing/data_driven_testing/data_driven_test.h"

#include "base/files/file_enumerator.h"
#include "base/files/file_util.h"
Expand All @@ -11,7 +11,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/re2/src/re2/re2.h"

namespace autofill {
namespace testing {
namespace {

// Reads |file| into |content|, and converts Windows line-endings to Unix ones.
Expand Down Expand Up @@ -58,13 +58,10 @@ void DataDrivenTest::RunDataDrivenTest(
base::ScopedAllowBlockingForTesting allow_blocking;
ASSERT_TRUE(base::DirectoryExists(input_directory));
ASSERT_TRUE(base::DirectoryExists(output_directory));
base::FileEnumerator input_files(input_directory,
false,
base::FileEnumerator::FILES,
file_name_pattern);
base::FileEnumerator input_files(
input_directory, false, base::FileEnumerator::FILES, file_name_pattern);
const bool kIsExpectedToPass = true;
for (base::FilePath input_file = input_files.Next();
!input_file.empty();
for (base::FilePath input_file = input_files.Next(); !input_file.empty();
input_file = input_files.Next()) {
RunOneDataDrivenTest(input_file, output_directory, kIsExpectedToPass);
}
Expand Down Expand Up @@ -111,25 +108,26 @@ void DataDrivenTest::RunOneDataDrivenTest(
}
}

base::FilePath DataDrivenTest::GetInputDirectory(
const base::FilePath::StringType& test_name) {
return test_data_directory_.AppendASCII("autofill")
.Append(test_name)
base::FilePath DataDrivenTest::GetInputDirectory() {
return test_data_directory_.Append(feature_directory_)
.Append(test_name_)
.AppendASCII("input");
}

base::FilePath DataDrivenTest::GetOutputDirectory(
const base::FilePath::StringType& test_name) {
return test_data_directory_.AppendASCII("autofill")
.Append(test_name)
base::FilePath DataDrivenTest::GetOutputDirectory() {
return test_data_directory_.Append(feature_directory_)
.Append(test_name_)
.AppendASCII("output");
}

DataDrivenTest::DataDrivenTest(const base::FilePath& test_data_directory)
: test_data_directory_(test_data_directory) {
}
DataDrivenTest::DataDrivenTest(
const base::FilePath& test_data_directory,
const base::FilePath::StringType& feature_directory,
const base::FilePath::StringType& test_name)
: test_data_directory_(test_data_directory),
feature_directory_(feature_directory),
test_name_(test_name) {}

DataDrivenTest::~DataDrivenTest() {
}
DataDrivenTest::~DataDrivenTest() {}

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

#ifndef COMPONENTS_AUTOFILL_CORE_BROWSER_DATA_DRIVEN_TEST_H_
#define COMPONENTS_AUTOFILL_CORE_BROWSER_DATA_DRIVEN_TEST_H_
#ifndef TESTING_DATA_DRIVEN_TESTING_DATA_DRIVEN_TEST_H_
#define TESTING_DATA_DRIVEN_TESTING_DATA_DRIVEN_TEST_H_

#include <string>

#include "base/files/file_path.h"
#include "base/macros.h"

namespace autofill {
namespace testing {

// A convenience class for implementing data-driven tests. Subclassers need only
// implement the conversion of serialized input data to serialized output data
// and provide a set of input files. For each input file, on the first run, a
// gold output file is generated; for subsequent runs, the test ouptut is
// gold output file is generated; for subsequent runs, the test output is
// compared to this gold output.
class DataDrivenTest {
public:
Expand Down Expand Up @@ -45,19 +45,22 @@ class DataDrivenTest {
std::string* output) = 0;

// Return |base::FilePath|s to the test input and output subdirectories
// ../autofill/|test_name|/input and ../autofill/|test_name|/output.
base::FilePath GetInputDirectory(const base::FilePath::StringType& test_name);
base::FilePath GetOutputDirectory(
const base::FilePath::StringType& test_name);
// ../|feature_dir|/|test_name|/input and ../|feature_dir|/|test_name|/output.
base::FilePath GetInputDirectory();
base::FilePath GetOutputDirectory();

protected:
DataDrivenTest(const base::FilePath& test_data_directory);
DataDrivenTest(const base::FilePath& test_data_directory,
const base::FilePath::StringType& feature_name,
const base::FilePath::StringType& test_name);
virtual ~DataDrivenTest();

private:
base::FilePath test_data_directory_;
base::FilePath::StringType feature_directory_;
base::FilePath::StringType test_name_;
};

} // namespace autofill
} // namespace testing

#endif // COMPONENTS_AUTOFILL_CORE_BROWSER_DATA_DRIVEN_TEST_H_
#endif // TESTING_DATA_DRIVEN_TESTING_DATA_DRIVEN_TEST_H_

0 comments on commit 945148b

Please sign in to comment.