This CL does the following: (1) Pass the max_length attribute to the password generator; (2) Update…

chrome/browser/autofill/password_generator_unittest.cc

 // Copyright (c) 2012 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.
 
 #include <locale>
 
 #include "chrome/browser/autofill/password_generator.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace autofill {
 
 TEST(PasswordGeneratorTest, PasswordGeneratorSimpleTest) {
-  // Not much to test, just make sure that the characters in a generated
-  // password are reasonable.
   PasswordGenerator pg;
-  std::string password = pg.Generate();
+  // Check max_length.

[Ilya Sherman, 2012/05/30 00:04:32]

nit: Please separate this out to a separate named test.

[Ilya Sherman, 2012/05/30 00:04:32]

nit: Please add a test case for max_length < 0.

[zysxqn, 2012/05/31 21:54:08]

Done.

[zysxqn, 2012/05/31 21:54:08]

Done.

+  std::string password = pg.Generate(10);
+  EXPECT_TRUE(password.size() == 10);
+  password = pg.Generate(0);
+  EXPECT_TRUE(password.size() == 12);

[Ilya Sherman, 2012/05/30 00:04:32]

nit: 12 is a magic number here.  If you need to use this constant in both the
implementation file and the test file, you should export it in the header
(probably in the private: section, with this test listed as a friend).

[zysxqn, 2012/05/31 21:54:08]

Seems that it's easier to just define that constant in the header file so both
test and .cc files can access it.

+  // Check password.

[Ilya Sherman, 2012/05/30 00:04:32]

nit: Please separate this out to a separate named test.

[zysxqn, 2012/05/31 21:54:08]

Done.

+  int num_upper_case_letters;
+  int num_lower_case_letters;
+  int num_digits;
+  int num_other_symbols;
   for (size_t i = 0; i < password.size(); i++) {
     // Make sure that the character is printable.
     EXPECT_TRUE(isgraph(password[i]));
+    if (isupper(password[i])) {
+      ++num_upper_case_letters;
+    } else if (islower(password[i])) {
+      ++num_lower_case_letters;
+    } else if (isdigit(password[i])) {
+      ++num_digits;
+    } else {
+      ++num_other_symbols;
+    }

[Ilya Sherman, 2012/05/30 00:04:32]

nit: No need for curly braces, since these are all one-line if-stmts.

[zysxqn, 2012/05/31 21:54:08]

Done.

   }
+  EXPECT_TRUE(num_upper_case_letters > 0);

[Ilya Sherman, 2012/05/30 00:04:32]

nit: EXPECT_GT

[zysxqn, 2012/05/31 21:54:08]

Done.

+  EXPECT_TRUE(num_lower_case_letters > 0);
+  EXPECT_TRUE(num_digits > 0);
+  EXPECT_TRUE(num_other_symbols > 0);
 }
 
 }  // namespace autofill