Skip to content

Commit

Permalink
[M105] Revert "diagnostics: enable log controller flag by default"
Browse files Browse the repository at this point in the history 
This reverts commit 047aacb.

Reason for revert: Unexpected behavior when Diagnostic App is open before locking screen.

Original change's description:
> diagnostics: enable log controller flag by default
>
> - Enables flag for Diagnostics Log Controller by default.
> - Update SessionLogHander tests to use NoSessionAshTestBase so that
> ash::Shell is correctly configured.
>
> Bug: b:225191282
> Test: Ran ash_unittests and ash_webui_unittests. Deployed code to DUT. Confirmed logs created in expected location and session log can still be generated.
> Change-Id: I5168bbf9511dfdf6bbd791f2d52ff6b7f20d98db
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3638605
> Reviewed-by: Gavin Williams <[email protected]>
> Commit-Queue: Ashley Prasad <[email protected]>
> Reviewed-by: Jimmy Gong <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1004294}

(cherry picked from commit b017569)

Bug: b:225191282
Change-Id: Ibe9cf2f90ec5c9f13dbeeb789d4e915698bd5327
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3783614
Reviewed-by: Jimmy Gong <[email protected]>
Commit-Queue: Ashley Prasad <[email protected]>
Cr-Original-Commit-Position: refs/heads/main@{#1027782}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3785048
Bot-Commit: Rubber Stamper <[email protected]>
Cr-Commit-Position: refs/branch-heads/5195@{#30}
Cr-Branched-From: 7aa3f07-refs/heads/main@{#1027018}
  • Loading branch information
Ashley Prasad authored and Chromium LUCI CQ committed Jul 26, 2022
1 parent 7d1baab commit 00ad85d
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 31 deletions.
2 changes: 1 addition & 1 deletion ash/constants/ash_features.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ const base::Feature kEnableLocalSearchService{"EnableLocalSearchService",
// diagnostics app routines, network events, and system snapshot.
// TODO(ashleydp): Remove this after the feature is launched.
const base::Feature kEnableLogControllerForDiagnosticsApp{
"EnableLogControllerForDiagnosticsApp", base::FEATURE_ENABLED_BY_DEFAULT};
"EnableLogControllerForDiagnosticsApp", base::FEATURE_DISABLED_BY_DEFAULT};

// If enabled, the networking cards will be shown in the diagnostics app.
const base::Feature kEnableNetworkingInDiagnosticsApp{
Expand Down
97 changes: 67 additions & 30 deletions ash/webui/diagnostics_ui/backend/session_log_handler_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ namespace diagnostics {
namespace {

constexpr char kHandlerFunctionName[] = "handlerFunctionName";
constexpr char kRoutineLogFileName[] = "diagnostic_routine_log";

mojom::SystemInfoPtr CreateSystemInfoPtr(const std::string& board_name,
const std::string& marketing_name,
Expand Down Expand Up @@ -168,49 +169,38 @@ class TestSelectFileDialogFactory : public ui::SelectFileDialogFactory {
base::FilePath selected_path_;
};

// Test class using NoSessionAshTestBase to ensure shell is available for
// tests requiring DiagnosticsLogController singleton.
class SessionLogHandlerTest : public NoSessionAshTestBase {
class SessionLogHandlerTest : public testing::Test {
public:
SessionLogHandlerTest()
: NoSessionAshTestBase(
base::test::TaskEnvironment::TimeSource::MOCK_TIME),
: task_environment_(),
task_runner_(new base::TestSimpleTaskRunner()),
web_ui_(),
session_log_handler_() {}
~SessionLogHandlerTest() override = default;

void SetUp() override {
session_log_handler_() {
EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
// Setup to ensure ash::Shell can configure for tests.
ui::ResourceBundle::CleanupSharedInstance();
AshTestSuite::LoadTestResources();
NoSessionAshTestBase::SetUp();
DiagnosticsLogController::Initialize(
std::make_unique<FakeDiagnosticsBrowserDelegate>());
auto* controller = DiagnosticsLogController::Get();
telemetry_log_ = controller->GetTelemetryLog();
routine_log_ = controller->GetRoutineLog();
networking_log_ = controller->GetNetworkingLog();
base::FilePath routine_log_path =
temp_dir_.GetPath().AppendASCII(kRoutineLogFileName);
auto telemetry_log = std::make_unique<TelemetryLog>();
auto routine_log = std::make_unique<RoutineLog>(routine_log_path);
auto networking_log = std::make_unique<NetworkingLog>(temp_dir_.GetPath());
telemetry_log_ = telemetry_log.get();
routine_log_ = routine_log.get();
networking_log_ = networking_log.get();
session_log_handler_ = std::make_unique<diagnostics::SessionLogHandler>(
base::BindRepeating(&CreateTestSelectFilePolicy),
/*telemetry_log*/ nullptr, /*routine_log*/ nullptr,
/*networking_log*/ nullptr, &holding_space_client_);
std::move(telemetry_log), std::move(routine_log),
std::move(networking_log), &holding_space_client_);
session_log_handler_->SetWebUIForTest(&web_ui_);
session_log_handler_->RegisterMessages();
session_log_handler_->SetTaskRunnerForTesting(task_runner_);

// Call handler to enable Javascript.
base::ListValue args;
web_ui_.HandleReceivedMessage("initialize", &args);
}

void TearDown() override {
~SessionLogHandlerTest() override {
task_runner_.reset();
task_environment()->RunUntilIdle();
task_environment_.RunUntilIdle();
ui::SelectFileDialog::SetFactory(nullptr);

NoSessionAshTestBase::TearDown();
}

void RunTasks() { task_runner_->RunPendingTasks(); }
Expand All @@ -224,23 +214,26 @@ class SessionLogHandlerTest : public NoSessionAshTestBase {
}

protected:
base::test::TaskEnvironment task_environment_{
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
// Task runner for tasks posted by save session log handler.
scoped_refptr<base::TestSimpleTaskRunner> task_runner_;

content::TestWebUI web_ui_;
std::unique_ptr<SessionLogHandler> session_log_handler_;
base::ScopedTempDir temp_dir_;
std::unique_ptr<diagnostics::SessionLogHandler> session_log_handler_;
TelemetryLog* telemetry_log_;
RoutineLog* routine_log_;
NetworkingLog* networking_log_;
testing::NiceMock<ash::MockHoldingSpaceClient> holding_space_client_;
base::ScopedTempDir temp_dir_;
};

// Flaky; see crbug.com/1336726
TEST_F(SessionLogHandlerTest, DISABLED_SaveSessionLog) {
base::RunLoop run_loop;
// Populate routine log
routine_log_->LogRoutineStarted(mojom::RoutineType::kCpuStress);
task_environment()->RunUntilIdle();
task_environment_.RunUntilIdle();

// Populate telemetry log
const std::string expected_board_name = "board_name";
Expand Down Expand Up @@ -311,8 +304,52 @@ TEST_F(SessionLogHandlerTest, DISABLED_SaveSessionLog) {
EXPECT_EQ("--- Network Events ---", log_lines[17]);
}

// Test class using NoSessionAshTestBase to ensure shell is available for
// tests requiring DiagnosticsLogController singleton.
class SessionLogHandlerAshTest : public NoSessionAshTestBase {
public:
SessionLogHandlerAshTest() : task_runner_(new base::TestSimpleTaskRunner()) {}
~SessionLogHandlerAshTest() override = default;

void SetUp() override {
// Setup to ensure ash::Shell can configure for tests.
ui::ResourceBundle::CleanupSharedInstance();
AshTestSuite::LoadTestResources();
// Setup feature list before setting up ash::Shell.
feature_list_.InitAndEnableFeature(
ash::features::kEnableLogControllerForDiagnosticsApp);
NoSessionAshTestBase::SetUp();
DiagnosticsLogController::Initialize(
std::make_unique<FakeDiagnosticsBrowserDelegate>());
session_log_handler_ = std::make_unique<SessionLogHandler>(
base::BindRepeating(&CreateTestSelectFilePolicy),
/*telemetry_log*/ nullptr,
/*routine_log*/ nullptr,
/*networking_log*/ nullptr,
/*holding_space_client*/ &holding_space_client_);
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
session_log_handler_->SetWebUIForTest(&web_ui_);
session_log_handler_->RegisterMessages();
session_log_handler_->SetTaskRunnerForTesting(task_runner_);
// Call handler to enable Javascript.
base::ListValue args;
web_ui_.HandleReceivedMessage("initialize", &args);
}

void RunTasks() { task_runner_->RunPendingTasks(); }

protected:
base::test::ScopedFeatureList feature_list_;
std::unique_ptr<SessionLogHandler> session_log_handler_;
content::TestWebUI web_ui_;
base::ScopedTempDir temp_dir_;
testing::NiceMock<ash::MockHoldingSpaceClient> holding_space_client_;
// Task runner for tasks posted by save session log handler.
scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
};

// Validates behavior when log controller is used to generate session log.
TEST_F(SessionLogHandlerTest, SaveHeaderOnlySessionLog) {
TEST_F(SessionLogHandlerAshTest, SaveSessionLogFlagEnabled) {
base::RunLoop run_loop;

// Simulate select file
Expand Down

0 comments on commit 00ad85d

Please sign in to comment.