New issue
Advanced search Search tips

Issue 912704 link

Starred by 1 user
Status: Started
Owner:
jdufault@chromium.org
Cc:
xiy...@chromium.org
r...@chromium.org
alemate@chromium.org
michae...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Componentize oobe and login browser tests

Project Member Reported by jdufault@chromium.org, Dec 6 
At the moment OOBE and login browser tests use a very deep inheritance hierarchy. It is hard to understand what a test is doing and it is hard to modify the base class because so many child tests depend on it in different ways.

Let's eliminate this inheritance and write reusable test components which a specific test injects. Then an OOBE test inherits directly from InProcessBrowserTests and adds the test components it needs. It is then immediately obvious what the test is mocking out and what it needs to run. It is then also easy to write a test that mocks out only xxx component.

Comment 1 by weifangsun@chromium.org, Dec 7

Components: UI>Shell>OOBE
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 12 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bd1d3787bc8da6057e7afe0c4e477c545558e1e3

commit bd1d3787bc8da6057e7afe0c4e477c545558e1e3
Author: Jacob Dufault <jdufault@google.com>
Date: Wed Dec 12 18:27:28 2018

cros: Directly construct JSChecker as needed

Eliminates a dependency on test bases

Bug: 912704
Change-Id: I30807d081cb790ff0f5c4c0873d75d9267c24b47
Reviewed-on: https://chromium-review.googlesource.com/c/1370908
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615972}
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/accessibility/spoken_feedback_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/active_directory_login_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/demo_mode/demo_setup_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/enable_debugging_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/enterprise_enrollment_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/eula_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/kiosk_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/login_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/login_manager_test.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/login_manager_test.h
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/login_ui_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/login_ui_keyboard_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/oobe_interactive_ui_test.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/oobe_localization_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/reset_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/saml/saml_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/screens/user_selection_screen_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/signin/device_id_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/test/js_checker.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/test/js_checker.h
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/test/oobe_base_test.cc
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/test/oobe_base_test.h
[modify] https://crrev.com/bd1d3787bc8da6057e7afe0c4e477c545558e1e3/chrome/browser/chromeos/login/webview_login_browsertest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 18 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4ba07b5a8c1c0a7f9cf24fccd9c5bada1d208805

commit 4ba07b5a8c1c0a7f9cf24fccd9c5bada1d208805
Author: Jacob Dufault <jdufault@google.com>
Date: Tue Dec 18 20:52:24 2018

cros: Remove LoginManagerTest::web_contents

Eliminates dependency on test base.

Bug: 912704
Change-Id: I73d210f3cd6c044a89168c796cca8c7e1bca5a78
Reviewed-on: https://chromium-review.googlesource.com/c/1374670
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617612}
[modify] https://crrev.com/4ba07b5a8c1c0a7f9cf24fccd9c5bada1d208805/chrome/browser/chromeos/login/demo_mode/demo_setup_browsertest.cc
[modify] https://crrev.com/4ba07b5a8c1c0a7f9cf24fccd9c5bada1d208805/chrome/browser/chromeos/login/enable_debugging_browsertest.cc
[modify] https://crrev.com/4ba07b5a8c1c0a7f9cf24fccd9c5bada1d208805/chrome/browser/chromeos/login/login_browsertest.cc
[modify] https://crrev.com/4ba07b5a8c1c0a7f9cf24fccd9c5bada1d208805/chrome/browser/chromeos/login/login_manager_test.cc
[modify] https://crrev.com/4ba07b5a8c1c0a7f9cf24fccd9c5bada1d208805/chrome/browser/chromeos/login/login_manager_test.h
[modify] https://crrev.com/4ba07b5a8c1c0a7f9cf24fccd9c5bada1d208805/chrome/browser/chromeos/login/reset_browsertest.cc
[modify] https://crrev.com/4ba07b5a8c1c0a7f9cf24fccd9c5bada1d208805/chrome/browser/chromeos/login/screens/user_selection_screen_browsertest.cc
[modify] https://crrev.com/4ba07b5a8c1c0a7f9cf24fccd9c5bada1d208805/chrome/browser/chromeos/login/test/js_checker.cc
[modify] https://crrev.com/4ba07b5a8c1c0a7f9cf24fccd9c5bada1d208805/chrome/browser/chromeos/login/test/js_checker.h
Project Member

Comment 5 by bugdroid, Today (9 hours ago) 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/45b69950e4871a10a3f50c6e5b4577c635ac1a17

commit 45b69950e4871a10a3f50c6e5b4577c635ac1a17
Author: Jacob Dufault <jdufault@google.com>
Date: Tue Jan 22 22:06:27 2019

cros: Remove set_issue_oauth_code_cookie in login_manager_test

Bug: 912704
Change-Id: I85386df408fb2a6365034dc65680ccef78553f02
Reviewed-on: https://chromium-review.googlesource.com/c/1376351
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624956}
[modify] https://crrev.com/45b69950e4871a10a3f50c6e5b4577c635ac1a17/chrome/browser/chromeos/login/login_manager_test.cc
[modify] https://crrev.com/45b69950e4871a10a3f50c6e5b4577c635ac1a17/chrome/browser/chromeos/login/login_manager_test.h
►

Sign in to add a comment