New issue
Advanced search Search tips

Issue 700512 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Task



Sign in to add a comment

[Autofill] Rewrite test cases to use INSTANTIATE_TEST_CASE_P

Project Member Reported by ma...@chromium.org, Mar 10 2017

Issue description

There are various places in the Autofill components_unittests where something like this is done:

...
  const struct {
    const char* input;
    bool expected_result;
  } kTestCases[] = {
      {"ab", true},
      {"abc", true},
      {"abde", true}
  };

  for (const auto& test_case : kTestCases) {
    EXPECT_EQ(test_case.expected_result,
              SomeFunction(test_case.input));
  }
... (this example is from [1])

This should rather use the INSTANTIATE_TEST_CASE_P mechanism that we have for tests. It will make 1 test for each test case. An example of that is here [2]. 

The full list of pathological cases can be gleaned from [3] 

[1] https://cs.chromium.org/chromium/src/components/autofill/core/common/autofill_util_unittest.cc?rcl=587f430144e9a2ad778649dff1c297b09977c2a3&l=28

[2] https://cs.chromium.org/chromium/src/components/autofill/core/browser/validation_unittest.cc?rcl=587f430144e9a2ad778649dff1c297b09977c2a3&l=189


[3] Some cases can be found here https://cs.chromium.org/search/?q=file:autofill.%2Bunittest.cc+struct.%2B%7B&type=cs
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/23ff72c5f2fc95e16f8eb996767d614976605193

commit 23ff72c5f2fc95e16f8eb996767d614976605193
Author: wuandy <wuandy@google.com>
Date: Tue Mar 14 21:41:02 2017

Rewrite Autofill unitttests under components to use INSTANTIATE_TEST_CASE_P

Many tests under component are like this:
const struct {
    const char* input;
    bool expected_result;
  } kTestCases[] = {
      {"ab", true},
      {"abc", true},
      {"abde", true}
  };

  for (const auto& test_case : kTestCases) {
    EXPECT_EQ(test_case.expected_result,
              SomeFunction(test_case.input));
  }

They should be rewritten using INSTANTIATE_TEST_CASE_P, it will create separate test cases for each testing input given.

BUG= 700512 
TEST=components_unittests

Review-Url: https://codereview.chromium.org/2744933004
Cr-Commit-Position: refs/heads/master@{#456843}

[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/address_i18n_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/autofill_data_model_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/autofill_data_util_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/autofill_field_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/contact_info_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/credit_card_field.h
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/credit_card_field_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/credit_card_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/personal_data_manager.h
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/personal_data_manager_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/phone_number_i18n_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/ui/card_unmask_prompt_controller_impl_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/webdata/autofill_profile_syncable_service_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/browser/webdata/autofill_table_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/common/autofill_regexes_unittest.cc
[modify] https://crrev.com/23ff72c5f2fc95e16f8eb996767d614976605193/components/autofill/core/common/autofill_util_unittest.cc

Comment 2 by ma...@chromium.org, Mar 15 2017

Owner: se...@chromium.org
Status: Assigned (was: Available)
Are we done or is there some cleanup?

Comment 3 by se...@chromium.org, Jul 26 2017

Status: Fixed (was: Assigned)

Sign in to add a comment