Add a data-driven unit test to validate autofill profile merging during aggregation.

Add a data-driven unit test to validate autofill profile merging during aggregation.

BUG=none
TEST=unit_tests --gtest_filter=AutoFillMergeTest.*

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=74210

[Ilya Sherman, 2011-02-05 05:46:07]

@dyu: Please take a look at the python scripts and let me know if I'm Doin' It
Wrong(TM) -- these are my first python scripts for Chromium.

[dhollowa, 2011-02-07 04:09:29]

LGTM.  Great stuff!  Some nits and a small data format change.

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
File chrome/browser/autofill/autofill_common_test.cc (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
chrome/browser/autofill/autofill_common_test.cc:89: if (profile) {
Chrome style seems to be:
if (!profile)
  return;

I'm ok with either.

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
File chrome/browser/autofill/autofill_merge_unittest.cc (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
chrome/browser/autofill/autofill_merge_unittest.cc:59: result +=
kProfileSeparator;
Separator preceding types please.  That's the yaml way.

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
chrome/browser/autofill/autofill_merge_unittest.cc:150: // Verify that the test
is idempotent on the output profiles .
nit: remove extra space before the "."

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
File chrome/browser/autofill/autofill_type.cc (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
chrome/browser/autofill/autofill_type.cc:8: #include "base/logging.h"
Is logging.h needed?

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/data...
File chrome/browser/autofill/data_driven_test.cc (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/data...
chrome/browser/autofill/data_driven_test.cc:37: &input));
nit: |input| doesn't need to wrap.

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
File chrome/test/data/autofill/merge/input/identical.in (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
chrome/test/data/autofill/merge/input/identical.in:1: NAME_FIRST:John
---
stuff
---
more stuff

[dyu1, 2011-02-07 20:48:08]

LGTM except field names comment.

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
File chrome/test/data/autofill/merge/tools/autofill_merge_common.py (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
chrome/test/data/autofill/merge/tools/autofill_merge_common.py:70: field_type =
"PHONE_HOME_NUMBER"
Should the field type name here be PHONE_HOME_WHOLE_NUMBER?

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
chrome/test/data/autofill/merge/tools/autofill_merge_common.py:72: field_type =
"PHONE_FAX_NUMBER"
PHONE_FAX_WHOLE_NUMBER?

[Ilya Sherman, 2011-02-08 05:31:40]

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
File chrome/browser/autofill/autofill_common_test.cc (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
chrome/browser/autofill/autofill_common_test.cc:89: if (profile) {
On 2011/02/07 04:09:29, dhollowa wrote:
> Chrome style seems to be:
> if (!profile)
>   return;
> 
> I'm ok with either.

I prefer to keep this style, since the first block of code in this function
makes sense even when the |profile| is NULL, and the block within the if-stmt is
pretty short.

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
File chrome/browser/autofill/autofill_merge_unittest.cc (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
chrome/browser/autofill/autofill_merge_unittest.cc:59: result +=
kProfileSeparator;
On 2011/02/07 04:09:29, dhollowa wrote:
> Separator preceding types please.  That's the yaml way.

Done.

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
chrome/browser/autofill/autofill_merge_unittest.cc:150: // Verify that the test
is idempotent on the output profiles .
On 2011/02/07 04:09:29, dhollowa wrote:
> nit: remove extra space before the "."

Done.

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
File chrome/browser/autofill/autofill_type.cc (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/auto...
chrome/browser/autofill/autofill_type.cc:8: #include "base/logging.h"
On 2011/02/07 04:09:29, dhollowa wrote:
> Is logging.h needed?

Yes, for NOTREACHED()

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/data...
File chrome/browser/autofill/data_driven_test.cc (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/browser/autofill/data...
chrome/browser/autofill/data_driven_test.cc:37: &input));
On 2011/02/07 04:09:29, dhollowa wrote:
> nit: |input| doesn't need to wrap.

Done.

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
File chrome/test/data/autofill/merge/input/identical.in (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
chrome/test/data/autofill/merge/input/identical.in:1: NAME_FIRST:John
On 2011/02/07 04:09:29, dhollowa wrote:
> ---
> stuff
> ---
> more stuff

Done.

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
File chrome/test/data/autofill/merge/tools/autofill_merge_common.py (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
chrome/test/data/autofill/merge/tools/autofill_merge_common.py:70: field_type =
"PHONE_HOME_NUMBER"
On 2011/02/07 20:48:08, dyu1 wrote:
> Should the field type name here be PHONE_HOME_WHOLE_NUMBER?

Done.

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
chrome/test/data/autofill/merge/tools/autofill_merge_common.py:72: field_type =
"PHONE_FAX_NUMBER"
On 2011/02/07 20:48:08, dyu1 wrote:
> PHONE_FAX_WHOLE_NUMBER?

Done.

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
File chrome/test/data/autofill/merge/tools/translate_profile_dump.py (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
chrome/test/data/autofill/merge/tools/translate_profile_dump.py:19: #
TODO(isherman): This file could use a better name.
Any suggestions for a better name?

[dhollowa, 2011-02-08 15:10:27]

LGTM.  Thanks.

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
File chrome/test/data/autofill/merge/tools/translate_profile_dump.py (right):

http://codereview.chromium.org/6312174/diff/1001/chrome/test/data/autofill/me...
chrome/test/data/autofill/merge/tools/translate_profile_dump.py:19: #
TODO(isherman): This file could use a better name.
profile_dump.py

On 2011/02/08 05:31:41, Ilya Sherman wrote:
> Any suggestions for a better name?