chrome/browser/autofill/form_structure_browsertest.cc - Issue 5781005: Upgrade autofill interactive_ui_test from BasicFormStructure test to I18NForm...

Side by Side Diff: chrome/browser/autofill/form_structure_browsertest.cc

Issue 5781005: Upgrade autofill interactive_ui_test from BasicFormStructure test to I18NForm... (Closed) Base URL: http://src.chromium.org/svn/trunk/src/

Patch Set: '' Created 10 years ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch | Annotate | Revision Log

OLD	NEW
1 // Copyright (c) 2010 The Chromium Authors. All rights reserved.	1 // Copyright (c) 2010 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #include <vector>	5 #include <vector>

6	6

	7 #include "base/file_path.h"

	8 #include "base/path_service.h"

	9 #include "base/string_util.h"

	10 #include "base/utf_string_conversions.h"

7 #include "chrome/browser/autofill/autofill_manager.h"	11 #include "chrome/browser/autofill/autofill_manager.h"

8 #include "chrome/browser/autofill/form_structure.h"	12 #include "chrome/browser/autofill/form_structure.h"

9 #include "chrome/browser/tab_contents/tab_contents.h"	13 #include "chrome/browser/tab_contents/tab_contents.h"

10 #include "chrome/browser/ui/browser.h"	14 #include "chrome/browser/ui/browser.h"

	15 #include "chrome/common/chrome_paths.h"

11 #include "chrome/test/in_process_browser_test.h"	16 #include "chrome/test/in_process_browser_test.h"

12 #include "chrome/test/ui_test_utils.h"	17 #include "chrome/test/ui_test_utils.h"

13 #include "googleurl/src/gurl.h"	18 #include "googleurl/src/gurl.h"

14	19

15 // Test class for verifying proper form structure as determined by AutoFill	20 namespace {

16 // heuristics. After a test loads HTML content with a call to \|NavigateToURL\|	21

17 // the \|AutoFillManager\| associated with the tab contents is queried for the	22 FilePath GetInputFileDirectory() {
	dhollowa 2010/12/15 03:20:45 These functions should not be indented. See http: These functions should not be indented. See http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces vivianz 2010/12/15 23:31:37 On 2010/12/15 03:20:45, dhollowa wrote: Done. On 2010/12/15 03:20:45, dhollowa wrote: Done.
18 // form structures that were loaded and parsed.	23 FilePath test_data_dir_;

19 // These form structures are serialized to string form and compared with	24 PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir_);

20 // expected results.	25 test_data_dir_ = test_data_dir_.AppendASCII("autofill_heuristics")

	26 .AppendASCII("input");

	27 return test_data_dir_;

	28 }

	29

	30 FilePath GetOutputFileDirectory() {

	31 FilePath test_data_dir_;

	32 PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir_);

	33 test_data_dir_ = test_data_dir_.AppendASCII("autofill_heuristics")

	34 .AppendASCII("output");

	35 return test_data_dir_;

	36 }

	37

	38 // Write \|outputcontent\| to \|outputfile\|. Returns true on success.

	39 bool WriteFile(const FilePath& outputfile,
	dhollowa 2010/12/15 03:20:45 Let's rename \|outputfile\| to \|file\|, and \|outputco Let's rename \|outputfile\| to \|file\|, and \|outputcontent\| to \|content\|. vivianz 2010/12/15 23:31:37 On 2010/12/15 03:20:45, dhollowa wrote: Done. On 2010/12/15 03:20:45, dhollowa wrote: Done.
	40 const std::string& outputcontent) {

	41 int write_size = file_util::WriteFile(outputfile, outputcontent.c_str(),

	42 outputcontent.length());

	43 return write_size == static_cast<int>(outputcontent.length());

	44 }

	45

	46 // Convert \|input_str\| to URL format, and return the converted string.

	47 const std::string ConvertToURLFormat(std::string& input_str) {
	dhollowa 2010/12/15 03:20:45 Let's change the input parameter to: const std::st Let's change the input parameter to: const std::string& html We can simply return the concatenation of the "data:..." string with the html string. I.e. return std::string("data:text/html;charset=utf-8,") + html; vivianz 2010/12/15 23:31:37 On 2010/12/15 03:20:45, dhollowa wrote: Done. On 2010/12/15 03:20:45, dhollowa wrote: Done.
	48 const std::string find_html_header_str(

	49 "<!DOCTYPE html PUBLIC \"-//W3C//DTD HTML 4.01//EN\">\n"

	50 "<html>\n <head>\n <title></title>\n"

	51 " </head>\n <body>\n");

	52 const std::string replace_str("data:text/html;charset=utf-8,");

	53

	54 ReplaceSubstringsAfterOffset(&input_str, 0, find_html_header_str,

	55 replace_str);

	56 return input_str;

	57 }

	58 } // namespace

	59

	60 // Test class for verifying proper international form structure as determined
	dhollowa 2010/12/15 03:20:45 This code is not i18n-specific. Let's change this This code is not i18n-specific. Let's change this back to the original comment. vivianz 2010/12/15 23:31:37 On 2010/12/15 03:20:45, dhollowa wrote: remove al On 2010/12/15 03:20:45, dhollowa wrote: remove all the i18n related comment, let me know if this looks ok now?
	61 // by AutoFill heuristics. A test inputs each i18n form file

	62 // (form_[language_code].html) from input directory, then loads its HTML

	63 // content with a call to \|NavigateToURL\|, the \|AutoFillManager\| associated

	64 // with the tab contents is queried for the form structures that were loaded

	65 // and parsed. These form structures are serialized to string form. If this is

	66 // the first time test is run, a gold test result file is generated in output

	67 // directory, else the form structures are compared again the existing gold

	68 // result file.

21 class FormStructureBrowserTest : public InProcessBrowserTest {	69 class FormStructureBrowserTest : public InProcessBrowserTest {

22 public:	70 public:

23 FormStructureBrowserTest() {}	71 FormStructureBrowserTest() {}

24 virtual ~FormStructureBrowserTest() {}	72 virtual ~FormStructureBrowserTest() {}

25	73

26 protected:	74 protected:

27 // Returns a vector of form structure objects associated with the given	75 // Returns a vector of form structure objects associated with the given

28 // \|autofill_manager\|.	76 // \|autofill_manager\|.

29 const std::vector<FormStructure*>& GetFormStructures(	77 const std::vector<FormStructure*>& GetFormStructures(

30 const AutoFillManager& autofill_manager);	78 const AutoFillManager& autofill_manager);

(...skipping 13 matching lines...) Expand all Loading...
44 const AutoFillManager& autofill_manager) {	92 const AutoFillManager& autofill_manager) {

45 return autofill_manager.form_structures_.get();	93 return autofill_manager.form_structures_.get();

46 }	94 }

47	95

48 const std::string FormStructureBrowserTest::FormStructuresToString(	96 const std::string FormStructureBrowserTest::FormStructuresToString(

49 const std::vector<FormStructure*>& forms) {	97 const std::vector<FormStructure*>& forms) {

50 std::string forms_string;	98 std::string forms_string;

51 for (std::vector<FormStructure*>::const_iterator iter = forms.begin();	99 for (std::vector<FormStructure*>::const_iterator iter = forms.begin();

52 iter != forms.end();	100 iter != forms.end();

53 ++iter) {	101 ++iter) {

54 forms_string += (*iter)->source_url().spec();

55 forms_string += "\n";

56	102

57 for (std::vector<AutoFillField*>::const_iterator field_iter =	103 for (std::vector<AutoFillField*>::const_iterator field_iter =

58 (*iter)->begin();	104 (*iter)->begin();

59 field_iter != (*iter)->end();	105 field_iter != (*iter)->end();

60 ++field_iter) {	106 ++field_iter) {

61 // The field list is NULL-terminated. Exit loop when at the end.	107 // The field list is NULL-terminated. Exit loop when at the end.

62 if (!*field_iter)	108 if (!*field_iter)

63 break;	109 break;

64 forms_string += AutoFillFieldTypeToString((*field_iter)->type());	110 forms_string += AutoFillFieldTypeToString((*field_iter)->type());

65 forms_string += "\n";	111 forms_string += "\n";

(...skipping 89 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
155 return "CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR";	201 return "CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR";

156 case CREDIT_CARD_TYPE:	202 case CREDIT_CARD_TYPE:

157 return "CREDIT_CARD_TYPE";	203 return "CREDIT_CARD_TYPE";

158 case CREDIT_CARD_VERIFICATION_CODE:	204 case CREDIT_CARD_VERIFICATION_CODE:

159 return "CREDIT_CARD_VERIFICATION_CODE";	205 return "CREDIT_CARD_VERIFICATION_CODE";

160 case COMPANY_NAME:	206 case COMPANY_NAME:

161 return "COMPANY_NAME";	207 return "COMPANY_NAME";

162 default:	208 default:

163 NOTREACHED() << "Invalid AutoFillFieldType value.";	209 NOTREACHED() << "Invalid AutoFillFieldType value.";

164 }	210 }

165

166 return std::string();	211 return std::string();

167 }	212 }

168	213

169 IN_PROC_BROWSER_TEST_F(FormStructureBrowserTest, BasicFormStructure) {	214 IN_PROC_BROWSER_TEST_F(FormStructureBrowserTest, HTMLFiles) {

170 ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));	215 FilePath input_file_path = GetInputFileDirectory();

171 ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(	216 file_util::FileEnumerator input_file_enumerator(input_file_path,

172 browser(), GURL("data:text/html;charset=utf-8,"	217 false, file_util::FileEnumerator::FILES);
	dhollowa 2010/12/15 03:20:45 Let's add the filter ".html" to limit processing Let's add the filter ".html" to limit processing to html files only. vivianz 2010/12/15 23:31:37 On 2010/12/15 03:20:45, dhollowa wrote: Done. On 2010/12/15 03:20:45, dhollowa wrote: Done.
173 "<form action=\"http://www.google.com/\" method=\"POST\">"

174 "<label for=\"firstname\">First name:</label>"

175 " <input type=\"text\" id=\"firstname\"/><br />"

176 "<label for=\"lastname\">Last name:</label>"

177 " <input type=\"text\" id=\"lastname\" /><br />"

178 "<label for=\"address1\">Address line 1:</label>"

179 " <input type=\"text\" id=\"address1\" /><br />"

180 "<label for=\"address2\">Address line 2:</label>"

181 " <input type=\"text\" id=\"address2\" /><br />"

182 "<label for=\"city\">City:</label>"

183 " <input type=\"text\" id=\"city\" /><br />"

184 "</form>")));

185	218

186 ASSERT_NO_FATAL_FAILURE(ui_test_utils::ClickOnView(browser(),	219 for (input_file_path = input_file_enumerator.Next();

187 VIEW_ID_TAB_CONTAINER));	220 !input_file_path.empty();

188 ASSERT_TRUE(ui_test_utils::IsViewFocused(browser(),	221 input_file_path = input_file_enumerator.Next()) {

189 VIEW_ID_TAB_CONTAINER_FOCUS_VIEW));	222 std::string input_file_source;

190	223

191 AutoFillManager* autofill_manager =	224 ASSERT_TRUE(file_util::ReadFileToString(input_file_path,

192 browser()->GetSelectedTabContents()->GetAutoFillManager();	225 &input_file_source));

193 ASSERT_NE(static_cast<AutoFillManager*>(NULL), autofill_manager);	226 input_file_source = ConvertToURLFormat(input_file_source);

194 std::vector<FormStructure> forms = GetFormStructures(autofill_manager);

195 std::string expected("data:text/html;charset=utf-8,"

196 "<form action=\"http://www.google.com/\""

197 " method=\"POST\">"

198 "<label for=\"firstname\">First name:</label>"

199 " <input type=\"text\" id=\"firstname\"/><br />"

200 "<label for=\"lastname\">Last name:</label>"

201 " <input type=\"text\" id=\"lastname\" /><br />"

202 "<label for=\"address1\">Address line 1:</label>"

203 " <input type=\"text\" id=\"address1\" /><br />"

204 "<label for=\"address2\">Address line 2:</label>"

205 " <input type=\"text\" id=\"address2\" /><br />"

206 "<label for=\"city\">City:</label>"

207 " <input type=\"text\" id=\"city\" /><br />"

208 "</form>\n"

209 "NAME_FIRST\n"

210 "NAME_LAST\n"

211 "ADDRESS_HOME_LINE1\n"

212 "ADDRESS_HOME_LINE2\n"

213 "ADDRESS_HOME_CITY\n");

214	227

215 EXPECT_EQ(expected, FormStructureBrowserTest::FormStructuresToString(forms));	228 ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));

	229 ASSERT_NO_FATAL_FAILURE(ui_test_utils::NavigateToURL(

	230 browser(), GURL(UTF8ToUTF16(input_file_source))));
	Ilya Sherman 2010/12/15 10:58:32 nit: I don't believe we need UTF8ToUTF16 here. nit: I don't believe we need UTF8ToUTF16 here. vivianz 2010/12/15 23:31:37 On 2010/12/15 10:58:32, Ilya Sherman wrote: I thi On 2010/12/15 10:58:32, Ilya Sherman wrote: I think we do need this here, otherwise, the first non-ASCII character appear in label, GURL will fail to recognize it. Ilya Sherman 2010/12/16 00:24:03 The GURL documentation [1] indicates std::string i Show quoted text On 2010/12/15 23:31:37, vivianz wrote: > On 2010/12/15 10:58:32, Ilya Sherman wrote: > > I think we do need this here, otherwise, the first non-ASCII character appear in > label, GURL will fail to recognize it. The GURL documentation [1] indicates std::string input to the constructor will be parsed as UTF8 input. Are you finding that this is not the case? If so, we should let brettw know. [1] http://codesearch.google.com/codesearch/p?vert=chromium#OAMlx_jo-ck/src/googl... vivianz 2010/12/16 02:00:14 On 2010/12/16 00:24:03, Ilya Sherman wrote: actau On 2010/12/16 00:24:03, Ilya Sherman wrote: actaully you are right, i jump to conclusion with my previous testing, after remove the unicode convertion, the previous failure of GURL is no longer appear now with non-ASCII label name in html. the failure we got before is likely still cause by the BOM character in html not utf8 GURL. fixed
	231

	232 ASSERT_NO_FATAL_FAILURE(ui_test_utils::ClickOnView(browser(),

	233 VIEW_ID_TAB_CONTAINER));
	Ilya Sherman 2010/12/15 10:58:32 nit: Please preserve the indentation here. The se nit: Please preserve the indentation here. The second line should line up with the "b" from "browser()". Same applies below. vivianz 2010/12/15 23:31:37 On 2010/12/15 10:58:32, Ilya Sherman wrote: Done. On 2010/12/15 10:58:32, Ilya Sherman wrote: Done.
	234 ASSERT_TRUE(ui_test_utils::IsViewFocused(browser(),

	235 VIEW_ID_TAB_CONTAINER_FOCUS_VIEW));

	236

	237 AutoFillManager* autofill_manager =

	238 browser()->GetSelectedTabContents()->GetAutoFillManager();

	239 ASSERT_NE(static_cast<AutoFillManager*>(NULL), autofill_manager);

	240 std::vector<FormStructure> forms = GetFormStructures(autofill_manager);

	241

	242 FilePath output_file_directory = GetOutputFileDirectory();

	243 FilePath output_file_path = output_file_directory.Append(input_file_path

	244 .BaseName().StripTrailingSeparators().ReplaceExtension(

	245 FILE_PATH_LITERAL(".out")));
	Ilya Sherman 2010/12/15 10:58:32 nit: I would prefer the following formatting (each nit: I would prefer the following formatting (each indent is four spaces -- even if it doesn't look like it with this non-monospaced font :) FilePath output_file_path = output_file_directory.Append( input_file_path.BaseName().StripTrailingSeparators().ReplaceExtension( FILE_PATH_LITERAL(".out"))); vivianz 2010/12/15 23:31:37 On 2010/12/15 10:58:32, Ilya Sherman wrote: Done. On 2010/12/15 10:58:32, Ilya Sherman wrote: Done.
	246

	247 std::string output_file_source;

	248 if (file_util::ReadFileToString(output_file_path, &output_file_source)) {

	249 EXPECT_EQ(output_file_source,
	dhollowa 2010/12/15 15:24:01 I ran this through the trybots and we're getting f I ran this through the trybots and we're getting failures on Windows. Windows is reading newlines as "\r\n", whereas Mac and Linux are reading them as "\n". To fix this we should change line 111 to use platform specific line breaks when generating the expected string. [ RUN ] FormStructureBrowserTest.HTMLFiles e:\b\build\slave\win\build\src\chrome\browser\autofill\form_structure_browsertest.cc(242): error: Value of: FormStructureBrowserTest::FormStructuresToString(forms) Actual: "NAME_FIRST\nNAME_LAST\nCOMPANY_NAME\nADDRESS_HOME_LINE1\nADDRESS_HOME_LINE2\nADDRESS_HOME_CITY\nADDRESS_HOME_ZIP\nADDRESS_HOME_STATE\nEMAIL_ADDRESS\nPHONE_HOME_WHOLE_NUMBER\nPHONE_FAX_WHOLE_NUMBER\n" Expected: output_file_source Which is: "NAME_FIRST\r\nNAME_LAST\r\nCOMPANY_NAME\r\nADDRESS_HOME_LINE1\r\nADDRESS_HOME_LINE2\r\nADDRESS_HOME_CITY\r\nADDRESS_HOME_ZIP\r\nADDRESS_HOME_STATE\r\nEMAIL_ADDRESS\r\nPHONE_HOME_WHOLE_NUMBER\r\nPHONE_FAX_WHOLE_NUMBER\r\n" vivianz 2010/12/15 23:31:37 On 2010/12/15 15:24:01, dhollowa wrote: my bad, I On 2010/12/15 15:24:01, dhollowa wrote: my bad, I over-complicated the situation here. the only reason this happen is because we use "\n" as sepearator for the autofill labels in output file, it is nothing to do with the input html file, so I believe just replace the seperator with a space should fix the problem, am I wrong? vivianz 2010/12/15 23:31:37 On 2010/12/15 15:24:01, dhollowa wrote: my bad, I On 2010/12/15 15:24:01, dhollowa wrote: my bad, I over-complicated the situation here. the reason this test failed is cause we use "\n" as line sepearator in our test result file. it has nothing to do with the input html file. so as long as we use platform unanimous character " ", this error should be fixed. am I wrong? so I replaced line 111 "\n" with " ". let me know if this is ok?
	250 FormStructureBrowserTest::FormStructuresToString(forms));

	251

	252 } else {

	253 ASSERT_TRUE(WriteFile(output_file_path,

	254 FormStructureBrowserTest::FormStructuresToString(forms)));
	Ilya Sherman 2010/12/15 10:58:32 nit: Arguments to a function should be aligned. I nit: Arguments to a function should be aligned. In this case, since the second argument cannot be indented to line up with the first, the first should be moved down onto a separate line so that it can align with the second. vivianz 2010/12/15 23:31:37 On 2010/12/15 10:58:32, Ilya Sherman wrote: Done. On 2010/12/15 10:58:32, Ilya Sherman wrote: Done.
	255 }

	256 }

216 }	257 }

OLD	NEW

« no previous file with comments | « no previous file | chrome/test/data/autofill_heuristics/input/form_en.html » ('j') | chrome/test/data/autofill_heuristics/input/form_en.html » ('J')