-
Notifications
You must be signed in to change notification settings - Fork 895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test importing passwords from Chrome #82
Conversation
lgtm but @darkdh is the better reviewer, so I'll defer to him to approve or ask for changes. |
EXPECT_EQ("test@example.com", | ||
UTF16ToASCII(password.username_value)); | ||
EXPECT_EQ("testing123", | ||
UTF16ToASCII(password.password_value)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also need to test the case of Never Saved List
Should we close this PR? |
Waiting for test case update and I will approve |
I will provide an updated test case this week. |
ChromeImporter::ImportPasswords modifies the "Login Data" database every time it runs. This is a side effect of LoginDatabase::Init and is not easily fixed. As a result, the ImportPasswords test modifes the test "Login Data" file, which is under version control, every time it runs. Copying the test profile data to a tempdir prevents the tests from inappropriately modifying files under version control.
aa9d0fe
to
d9876f1
Compare
This PR now tests importing both "autofillable" and "blacklisted" logins from Chrome, as requested by @darkdh in his review. A login is autofillable or blacklisted depending on whether you click "Yes" or "Never" when Chrome prompts you to save a password. In addition, I improved the process for generating the test copy of Chrome's |
UTF16ToASCII(password.username_value)); | ||
EXPECT_EQ("testing123", | ||
UTF16ToASCII(password.password_value)); | ||
EXPECT_FALSE(autofillable_login.blacklisted_by_user); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we also verify signon_realm
for both autofillable_login
and blacklisted_login
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work. ++
don't allow multiple concurrent init calls
set stats to 99+ if resource is above that number
Ignore xtb files when creating patches
Ignore xtb files when creating patches
Ignore xtb files when creating patches
Unit tests for #77. Also refactors the importer unit tests to reduce code duplication.
Submitter Checklist:
git rebase -i
to squash commits (if needed).Test Plan:
yarn test brave_unit_tests
Reviewer Checklist: