Commit 2ecaaa59 authored by Maggie Cai's avatar Maggie Cai Committed by Commit Bot

Revert "Always inject mocks for more hemertic InputMethodManagerImpl unit tests."

This reverts commit 2b9c5c7c.

Reason for revert: This CL is likely the root cause for build failure for linux-chromeos-dbg, starting from https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-dbg/21474
The failed tests are: InputMethodManagerImplTest.SetLoginDefaultWithAllowedKeyboardLayouts
InputMethodManagerImplTest.TestEnableLayoutsNonUsHardwareKeyboard
InputMethodManagerImplTest.TestEnableMultipleHardwareKeyboardLayout

Original change's description:
> Always inject mocks for more hemertic InputMethodManagerImpl unit tests.
>
> Six InputMethodManagerImpl tests are failing in specific environments
> due to mixed usage of test data and CrOS prod configs. Partly caused by
> inconsistent usage of MockComponentExtensionIMEDelegateManager. In
> particular, parts of Setup() are run without this mock injected. The fix
> here ensures the mock and its test data are consistently used throughout.
>
> With this we manage to un-fail 4 out of 6 tests, but another new test
> starts failing! It's unsurprising because these tests still indirectly
> depend on deprecated input_methods.txt configs (remnant from before the
> migration to extension-based input methods circa 2011). This issue will
> be addressed in the next steps.
>
> Bug: 970790
> Change-Id: I3e76708e34f2b4e021f763654714d157f48a3925
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2550152
> Commit-Queue: Bao-Duy Tran <tranbaoduy@chromium.org>
> Reviewed-by: Yuly Novikov <ynovikov@chromium.org>
> Reviewed-by: John Palmer <jopalmer@chromium.org>
> Reviewed-by: Darren Shen <shend@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#831198}

TBR=ynovikov@chromium.org,shend@chromium.org,tranbaoduy@chromium.org,jopalmer@chromium.org

Change-Id: I4fea5ebf6f46f66ba837d693f7970a0ac8949054
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 970790
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2560527Reviewed-by: default avatarMaggie Cai <mxcai@chromium.org>
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831256}
parent 9e2a55c6
...@@ -1255,6 +1255,13 @@ void InputMethodManagerImpl::SetImeKeyboardForTesting(ImeKeyboard* keyboard) { ...@@ -1255,6 +1255,13 @@ void InputMethodManagerImpl::SetImeKeyboardForTesting(ImeKeyboard* keyboard) {
keyboard_.reset(keyboard); keyboard_.reset(keyboard);
} }
void InputMethodManagerImpl::InitializeComponentExtensionForTesting(
std::unique_ptr<ComponentExtensionIMEManagerDelegate> delegate) {
component_extension_ime_manager_->Initialize(std::move(delegate));
util_.ResetInputMethods(
component_extension_ime_manager_->GetAllIMEAsInputMethodDescriptor());
}
void InputMethodManagerImpl::Observe( void InputMethodManagerImpl::Observe(
int type, int type,
const content::NotificationSource& source, const content::NotificationSource& source,
......
...@@ -249,6 +249,9 @@ class InputMethodManagerImpl : public InputMethodManager, ...@@ -249,6 +249,9 @@ class InputMethodManagerImpl : public InputMethodManager,
CandidateWindowController* candidate_window_controller); CandidateWindowController* candidate_window_controller);
// Sets |keyboard_|. // Sets |keyboard_|.
void SetImeKeyboardForTesting(ImeKeyboard* keyboard); void SetImeKeyboardForTesting(ImeKeyboard* keyboard);
// Initialize |component_extension_manager_|.
void InitializeComponentExtensionForTesting(
std::unique_ptr<ComponentExtensionIMEManagerDelegate> delegate);
// content::NotificationObserver overrides: // content::NotificationObserver overrides:
void Observe(int type, void Observe(int type,
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
#include "chrome/browser/chromeos/input_method/component_extension_ime_manager_delegate_impl.h"
#include "chrome/browser/chromeos/input_method/mock_candidate_window_controller.h" #include "chrome/browser/chromeos/input_method/mock_candidate_window_controller.h"
#include "chrome/browser/chromeos/input_method/mock_input_method_engine.h" #include "chrome/browser/chromeos/input_method/mock_input_method_engine.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
...@@ -138,15 +139,9 @@ class InputMethodManagerImplTest : public BrowserWithTestWindowTest { ...@@ -138,15 +139,9 @@ class InputMethodManagerImplTest : public BrowserWithTestWindowTest {
void SetUp() override { void SetUp() override {
ui::InitializeInputMethodForTesting(); ui::InitializeInputMethodForTesting();
InitImeList();
auto mock_delegate =
std::make_unique<MockComponentExtensionIMEManagerDelegate>();
mock_delegate->set_ime_list(ime_list_);
manager_ = std::make_unique<InputMethodManagerImpl>( manager_ = std::make_unique<InputMethodManagerImpl>(
std::make_unique<FakeInputMethodDelegate>(), std::move(mock_delegate), std::make_unique<FakeInputMethodDelegate>(),
false); std::make_unique<ComponentExtensionIMEManagerDelegateImpl>(), false);
manager_->GetInputMethodUtil()->UpdateHardwareLayoutCache(); manager_->GetInputMethodUtil()->UpdateHardwareLayoutCache();
candidate_window_controller_ = new MockCandidateWindowController; candidate_window_controller_ = new MockCandidateWindowController;
manager_->SetCandidateWindowControllerForTesting( manager_->SetCandidateWindowControllerForTesting(
...@@ -159,12 +154,32 @@ class InputMethodManagerImplTest : public BrowserWithTestWindowTest { ...@@ -159,12 +154,32 @@ class InputMethodManagerImplTest : public BrowserWithTestWindowTest {
menu_manager_ = ui::ime::InputMethodMenuManager::GetInstance(); menu_manager_ = ui::ime::InputMethodMenuManager::GetInstance();
InitImeList();
BrowserWithTestWindowTest::SetUp(); BrowserWithTestWindowTest::SetUp();
// Needs ash::Shell keyboard to be created first. // Needs ash::Shell keyboard to be created first.
chrome_keyboard_controller_client_test_helper_ = chrome_keyboard_controller_client_test_helper_ =
ChromeKeyboardControllerClientTestHelper::InitializeForAsh(); ChromeKeyboardControllerClientTestHelper::InitializeForAsh();
InitComponentExtension();
}
void TearDown() override {
// Needs to destroyed before ash::Shell keyboard.
chrome_keyboard_controller_client_test_helper_.reset();
BrowserWithTestWindowTest::TearDown();
ui::ShutdownInputMethodForTesting();
candidate_window_controller_ = nullptr;
keyboard_ = nullptr;
manager_.reset();
}
private:
// Helper function to initialize component extension stuff for testing.
void InitComponentExtension() {
// CreateNewState(nullptr) returns state with non-empty // CreateNewState(nullptr) returns state with non-empty
// current_input_method. So SetState() triggers ChangeInputMethod(). // current_input_method. So SetState() triggers ChangeInputMethod().
InputMethodDescriptors descriptors; InputMethodDescriptors descriptors;
...@@ -177,18 +192,17 @@ class InputMethodManagerImplTest : public BrowserWithTestWindowTest { ...@@ -177,18 +192,17 @@ class InputMethodManagerImplTest : public BrowserWithTestWindowTest {
state->AddInputMethodExtension(extension_ime_util::kT13nExtensionId, state->AddInputMethodExtension(extension_ime_util::kT13nExtensionId,
descriptors, mock_engine_handler_.get()); descriptors, mock_engine_handler_.get());
manager_->SetState(state); manager_->SetState(state);
}
void TearDown() override {
// Needs to destroyed before ash::Shell keyboard.
chrome_keyboard_controller_client_test_helper_.reset();
BrowserWithTestWindowTest::TearDown(); MockComponentExtensionIMEManagerDelegate* mock_delegate =
ui::ShutdownInputMethodForTesting(); new MockComponentExtensionIMEManagerDelegate();
mock_delegate->set_ime_list(ime_list_);
std::unique_ptr<ComponentExtensionIMEManagerDelegate> delegate(
mock_delegate);
candidate_window_controller_ = nullptr; // Note, for production, these SetEngineHandler are called when
keyboard_ = nullptr; // IMEEngineHandlerInterface is initialized via
manager_.reset(); // InitializeComponentextension.
manager_->InitializeComponentExtensionForTesting(std::move(delegate));
} }
void InitImeList() { void InitImeList() {
......
...@@ -4402,9 +4402,6 @@ ...@@ -4402,9 +4402,6 @@
"test_id_prefix": "ninja://ui/touch_selection:ui_touch_selection_unittests/" "test_id_prefix": "ninja://ui/touch_selection:ui_touch_selection_unittests/"
}, },
{ {
"args": [
"--test-launcher-filter-file=../../testing/buildbot/filters/chromeos.unit_tests.filter"
],
"isolate_profile_data": true, "isolate_profile_data": true,
"merge": { "merge": {
"args": [], "args": [],
......
# TODO(crbug.com/970790): Enable this. # TODO(crbug.com/970790): Enable this.
-InputMethodManagerImplTest.SetLoginDefaultWithAllowedKeyboardLayouts -InputMethodManagerImplTest.TestEnableLayouts
-InputMethodManagerImplTest.TestEnableLayoutsNonUsHardwareKeyboard -InputMethodManagerImplTest.TestEnableLayoutsNonUsHardwareKeyboard
-InputMethodManagerImplTest.TestEnableMultipleHardwareKeyboardLayout -InputMethodManagerImplTest.TestEnableMultipleHardwareKeyboardLayout
-InputMethodManagerImplTest.TestLastUsedInputMethod
-InputMethodManagerImplTest.TestNextInputMethod
-InputMethodManagerImplTest.TestObserver
# TODO(crbug.com/970806): Enable this. # TODO(crbug.com/970806): Enable this.
-KeyboardShortcutViewerMetadataTest.ModifyAcceleratorShouldUpdateMetadata -KeyboardShortcutViewerMetadataTest.ModifyAcceleratorShouldUpdateMetadata
...@@ -2761,9 +2761,6 @@ ...@@ -2761,9 +2761,6 @@
}, },
}, },
'linux-chromeos-rel': { 'linux-chromeos-rel': {
'args': [
'--test-launcher-filter-file=../../testing/buildbot/filters/chromeos.unit_tests.filter',
],
'swarming': { 'swarming': {
'shards': 2, 'shards': 2,
}, },
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment