Currently, PasswordManager has no separation from its interface so when testing PasswordFormManager, the production version of PasswordManager needs to be used. This causes need for complex mocking (all dependencies of PasswordManager instead of PasswordManager itself), and after https://codereview.chromium.org/2263933002, some tests (UpdateFormManagers_IsCalled) are no longer even possible.
Instead, we need to specify at least the API used by PasswordFormManager as an abstract interface and use PasswordManager as its production implementation, while in the PasswordFormManagerTest we can have a mocked one.
Additional notes:
The UpdateFormManagers_IsCalled used to test that when PasswordFormManager::Save was called, then the PasswordFormManager also called PasswordManager::UpdateFormManagers/
Currently, PasswordManager has no separation from its interface so when testing PasswordFormManager, the production version of PasswordManager needs to be used. This causes need for complex mocking (all dependencies of PasswordManager instead of PasswordManager itself), and after https://codereview.chromium.org/2263933002, some tests (UpdateFormManagers_IsCalled) are no longer even possible.
Instead, we need to specify at least the API used by PasswordFormManager as an abstract interface and use PasswordManager as its production implementation, while in the PasswordFormManagerTest we can have a mocked one.
Additional notes:
The UpdateFormManagers_IsCalled used to test that when PasswordFormManager::Save was called, then the PasswordFormManager also called PasswordManager::UpdateFormManagers.
Currently, PasswordManager has no separation from its interface so when testing PasswordFormManager, the production version of PasswordManager needs to be used. This causes need for complex mocking (all dependencies of PasswordManager instead of PasswordManager itself), and after https://codereview.chromium.org/2263933002, some tests (UpdateFormManagers_IsCalled) are no longer even possible.
Similarly, PasswordFormManager lacks an API to be mocked when testing PasswordManager.
We need to abstract the public API from both classes and allow injecting mocked versions at least when testing PasswordManager and PasswordFormManager. Injecting a mocked PasswordManager into PasswordFormManager is easy (it is already a parameter on construction). The other direction is harder: PasswordManager creates its own instances of PasswordFormManager on demand; to ensure they are mocked, we might need to pull out creating them into a separate factory object.
Additional notes:
PasswordManager.UpdateFormManagers_IsCalled used to test that when PasswordFormManager::Save was called, then the PasswordFormManager also called PasswordManager::UpdateFormManagers.
Assigning to myself, as I'd like not to forget about doing this.
If bug 621355 is fixed and this one not, feel free to notify me and grab this from me, if you want.
Comment 1 by vabr@chromium.org
, Aug 22 2016