Session Log: Categorize routines
- Split routines to their categories
- Rename ambiguous headers
Bug: 1125150
Test: ash_webui_unittests
Change-Id: I18edde3a41c6613b1459e612868aa4cdb420bdba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3170926
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Commit-Queue: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/main@{#924145}
diff --git a/ash/webui/diagnostics_ui/backend/network_health_provider_unittest.cc b/ash/webui/diagnostics_ui/backend/network_health_provider_unittest.cc
index 97dc7934..cec0006 100644
--- a/ash/webui/diagnostics_ui/backend/network_health_provider_unittest.cc
+++ b/ash/webui/diagnostics_ui/backend/network_health_provider_unittest.cc
@@ -1378,7 +1378,7 @@
EXPECT_TRUE(list_observer.active_guid().empty());
// The non-active network still appears in the log.
- EXPECT_FALSE(log.GetContents().empty());
+ EXPECT_FALSE(log.GetNetworkInfo().empty());
// Put wifi into online state.
SetWifiOnline();
@@ -1386,7 +1386,7 @@
// Log is populated with network info now that WiFi is online.
// Log contents tested in networking_log_unittest.cc -
// NetworkingLogTest.DetailedLogContentsWiFi.
- EXPECT_FALSE(log.GetContents().empty());
+ EXPECT_FALSE(log.GetNetworkInfo().empty());
}
TEST_F(NetworkHealthProviderTest, ResetReceiverOnDisconnect) {
diff --git a/ash/webui/diagnostics_ui/backend/networking_log.cc b/ash/webui/diagnostics_ui/backend/networking_log.cc
index b4220ea..63ae9b6 100644
--- a/ash/webui/diagnostics_ui/backend/networking_log.cc
+++ b/ash/webui/diagnostics_ui/backend/networking_log.cc
@@ -23,7 +23,7 @@
const char kNewline[] = "\n";
// NetworkingInfo constants:
-const char kNetworkingInfoSectionName[] = "--- Networking Info ---";
+const char kNetworkingInfoSectionName[] = "--- Network Info ---";
const char kNetworkNameTitle[] = "Name: ";
const char kNetworkTypeTitle[] = "Type: ";
const char kNetworkStateTitle[] = "State: ";
@@ -177,7 +177,7 @@
NetworkingLog::NetworkingLog() = default;
NetworkingLog::~NetworkingLog() = default;
-std::string NetworkingLog::GetContents() const {
+std::string NetworkingLog::GetNetworkInfo() const {
std::stringstream output;
output << kNetworkingInfoSectionName << kNewline;
diff --git a/ash/webui/diagnostics_ui/backend/networking_log.h b/ash/webui/diagnostics_ui/backend/networking_log.h
index 799fa731..7b07b73 100644
--- a/ash/webui/diagnostics_ui/backend/networking_log.h
+++ b/ash/webui/diagnostics_ui/backend/networking_log.h
@@ -23,7 +23,7 @@
~NetworkingLog();
// Returns the networking log as a string.
- std::string GetContents() const;
+ std::string GetNetworkInfo() const;
// Updates the list of valid networks and which is active.
void UpdateNetworkList(const std::vector<std::string>& observer_guids,
diff --git a/ash/webui/diagnostics_ui/backend/networking_log_unittest.cc b/ash/webui/diagnostics_ui/backend/networking_log_unittest.cc
index f6daebd..db1857c 100644
--- a/ash/webui/diagnostics_ui/backend/networking_log_unittest.cc
+++ b/ash/webui/diagnostics_ui/backend/networking_log_unittest.cc
@@ -98,12 +98,12 @@
log.UpdateNetworkList({expected_guid}, expected_guid);
log.UpdateNetworkState(test_info.Clone());
- const std::string log_as_string = log.GetContents();
+ const std::string log_as_string = log.GetNetworkInfo();
const std::vector<std::string> log_lines = GetLogLines(log_as_string);
// Expect one title line and 14 content lines.
EXPECT_EQ(15u, log_lines.size());
- EXPECT_EQ("--- Networking Info ---", log_lines[0]);
+ EXPECT_EQ("--- Network Info ---", log_lines[0]);
EXPECT_EQ("Name: " + expected_name, log_lines[1]);
EXPECT_EQ("Type: WiFi", log_lines[2]);
EXPECT_EQ("State: Online", log_lines[3]);
@@ -139,12 +139,12 @@
log.UpdateNetworkList({expected_guid}, expected_guid);
log.UpdateNetworkState(test_info.Clone());
- const std::string log_as_string = log.GetContents();
+ const std::string log_as_string = log.GetNetworkInfo();
const std::vector<std::string> log_lines = GetLogLines(log_as_string);
// Expect one title line and 10 content lines.
EXPECT_EQ(11u, log_lines.size());
- EXPECT_EQ("--- Networking Info ---", log_lines[0]);
+ EXPECT_EQ("--- Network Info ---", log_lines[0]);
EXPECT_EQ("Name: " + expected_name, log_lines[1]);
EXPECT_EQ("Type: Ethernet", log_lines[2]);
EXPECT_EQ("State: Online", log_lines[3]);
@@ -166,12 +166,12 @@
log.UpdateNetworkList({expected_guid}, expected_guid);
log.UpdateNetworkState(test_info.Clone());
- const std::string log_as_string = log.GetContents();
+ const std::string log_as_string = log.GetNetworkInfo();
const std::vector<std::string> log_lines = GetLogLines(log_as_string);
// Expect one title line and 9 content lines.
EXPECT_EQ(10u, log_lines.size());
- EXPECT_EQ("--- Networking Info ---", log_lines[0]);
+ EXPECT_EQ("--- Network Info ---", log_lines[0]);
EXPECT_EQ("Name: " + expected_name, log_lines[1]);
EXPECT_EQ("Type: Cellular", log_lines[2]);
EXPECT_EQ("State: Online", log_lines[3]);
diff --git a/ash/webui/diagnostics_ui/backend/routine_log.cc b/ash/webui/diagnostics_ui/backend/routine_log.cc
index 1052a33..81a52359 100644
--- a/ash/webui/diagnostics_ui/backend/routine_log.cc
+++ b/ash/webui/diagnostics_ui/backend/routine_log.cc
@@ -53,9 +53,29 @@
// Get the category for the routine `type`.
std::string GetRoutineCategory(mojom::RoutineType type) {
switch (type) {
- // TODO(zentaro): Implement categorization.
- default:
- return "all";
+ case mojom::RoutineType::kBatteryCharge:
+ case mojom::RoutineType::kBatteryDischarge:
+ case mojom::RoutineType::kCpuCache:
+ case mojom::RoutineType::kCpuStress:
+ case mojom::RoutineType::kCpuFloatingPoint:
+ case mojom::RoutineType::kCpuPrime:
+ case mojom::RoutineType::kMemory:
+ return "system";
+ case mojom::RoutineType::kLanConnectivity:
+ case mojom::RoutineType::kSignalStrength:
+ case mojom::RoutineType::kGatewayCanBePinged:
+ case mojom::RoutineType::kHasSecureWiFiConnection:
+ case mojom::RoutineType::kDnsResolverPresent:
+ case mojom::RoutineType::kDnsLatency:
+ case mojom::RoutineType::kDnsResolution:
+ case mojom::RoutineType::kCaptivePortal:
+ case mojom::RoutineType::kHttpFirewall:
+ case mojom::RoutineType::kHttpsFirewall:
+ case mojom::RoutineType::kHttpsLatency:
+ case mojom::RoutineType::kArcHttp:
+ case mojom::RoutineType::kArcPing:
+ case mojom::RoutineType::kArcDnsResolution:
+ return "network";
};
}
@@ -92,9 +112,6 @@
std::string RoutineLog::GetContentsForCategory(
const std::string& category) const {
- // TODO(zentaro): Remove DCHECK once categories are implemented.
- DCHECK_EQ("all", category);
-
const auto iter = logs_.find(category);
if (iter == logs_.end()) {
return "";
@@ -106,9 +123,6 @@
void RoutineLog::Append(mojom::RoutineType type, const std::string& text) {
std::string category = GetRoutineCategory(type);
- // TODO(zentaro): Remove DCHECK once categories are implemented.
- DCHECK_EQ("all", category);
-
// Insert a new log if it doesn't exist then append to it.
base::FilePath log_path = GetCategoryLogFilePath(category);
auto iter = logs_.find(category);
diff --git a/ash/webui/diagnostics_ui/backend/routine_log_unittest.cc b/ash/webui/diagnostics_ui/backend/routine_log_unittest.cc
index 3d881c34..eaed95c 100644
--- a/ash/webui/diagnostics_ui/backend/routine_log_unittest.cc
+++ b/ash/webui/diagnostics_ui/backend/routine_log_unittest.cc
@@ -62,7 +62,7 @@
EXPECT_TRUE(base::PathExists(log_path_));
- const std::string contents = log.GetContentsForCategory("all");
+ const std::string contents = log.GetContentsForCategory("system");
const std::string first_line = GetLogLines(contents)[0];
const std::vector<std::string> first_line_contents =
GetLogLineContents(first_line);
@@ -84,7 +84,7 @@
EXPECT_TRUE(base::PathExists(log_path_));
- const std::string contents = log.GetContentsForCategory("all");
+ const std::string contents = log.GetContentsForCategory("system");
const std::vector<std::string> log_lines = GetLogLines(contents);
const std::string first_line = log_lines[0];
const std::vector<std::string> first_line_contents =
@@ -114,7 +114,7 @@
EXPECT_TRUE(base::PathExists(log_path_));
- const std::string contents = log.GetContentsForCategory("all");
+ const std::string contents = log.GetContentsForCategory("system");
LOG(ERROR) << contents;
const std::vector<std::string> log_lines = GetLogLines(contents);
diff --git a/ash/webui/diagnostics_ui/backend/session_log_handler.cc b/ash/webui/diagnostics_ui/backend/session_log_handler.cc
index 8a48f4e..d02d992 100644
--- a/ash/webui/diagnostics_ui/backend/session_log_handler.cc
+++ b/ash/webui/diagnostics_ui/backend/session_log_handler.cc
@@ -25,11 +25,23 @@
namespace diagnostics {
namespace {
-const char kRoutineLogSectionHeader[] = "=== Routine Log === \n";
-const char kTelemetryLogSectionHeader[] = "=== Telemetry Log === \n";
-const char kNetworkingLogSectionHeader[] = "=== Networking Log === \n";
+const char kRoutineLogSubsectionHeader[] = "--- Test Routines --- \n";
+const char kSystemLogSectionHeader[] = "=== System === \n";
+const char kNetworkingLogSectionHeader[] = "=== Networking === \n";
+const char kNoRoutinesRun[] =
+ "No routines of this type were run in the session.\n";
const char kDefaultSessionLogFileName[] = "session_log.txt";
-const char kRoutineLogPath[] = "/tmp/diagnostics/diagnostics_routine_log";
+const char kRoutineLogBasePath[] = "/tmp/diagnostics/";
+
+std::string GetRoutineResultsString(const std::string& results) {
+ const std::string section_header =
+ std::string(kRoutineLogSubsectionHeader) + "\n";
+ if (results.empty()) {
+ return section_header + kNoRoutinesRun;
+ }
+
+ return section_header + results;
+}
} // namespace
@@ -39,7 +51,7 @@
: SessionLogHandler(
select_file_policy_creator,
std::make_unique<TelemetryLog>(),
- std::make_unique<RoutineLog>(base::FilePath(kRoutineLogPath)),
+ std::make_unique<RoutineLog>(base::FilePath(kRoutineLogBasePath)),
std::make_unique<NetworkingLog>(),
holding_space_client) {}
@@ -121,22 +133,33 @@
}
bool SessionLogHandler::CreateSessionLog(const base::FilePath& file_path) {
- // Fetch RoutineLog
- // TODO(zentaro): Update when categories are supported.
- const std::string routine_log_contents =
- routine_log_->GetContentsForCategory("all");
+ // Fetch Routine logs
+ const std::string system_routines =
+ routine_log_->GetContentsForCategory("system");
+ const std::string network_routines =
+ routine_log_->GetContentsForCategory("network");
- // Fetch TelemetryLog
- const std::string telemetry_log_contents = telemetry_log_->GetContents();
+ // Fetch system data from TelemetryLog.
+ const std::string system_log_contents = telemetry_log_->GetContents();
- std::vector<std::string> pieces = {
- kTelemetryLogSectionHeader, telemetry_log_contents,
- kRoutineLogSectionHeader, routine_log_contents};
+ std::vector<std::string> pieces;
+ pieces.push_back(kSystemLogSectionHeader);
+ if (!system_log_contents.empty()) {
+ pieces.push_back(system_log_contents);
+ }
+
+ // Add the routine section for the system category.
+ pieces.push_back(GetRoutineResultsString(system_routines));
if (features::IsNetworkingInDiagnosticsAppEnabled()) {
- // Fetch NetworkingLog
+ // Add networking category.
pieces.push_back(kNetworkingLogSectionHeader);
- pieces.push_back(networking_log_->GetContents());
+
+ // Add the network info section.
+ pieces.push_back(networking_log_->GetNetworkInfo());
+
+ // Add the routine section for the network category.
+ pieces.push_back(GetRoutineResultsString(network_routines));
}
return base::WriteFile(file_path, base::JoinString(pieces, "\n"));
diff --git a/ash/webui/diagnostics_ui/backend/session_log_handler_unittest.cc b/ash/webui/diagnostics_ui/backend/session_log_handler_unittest.cc
index b46504c..a97862f2 100644
--- a/ash/webui/diagnostics_ui/backend/session_log_handler_unittest.cc
+++ b/ash/webui/diagnostics_ui/backend/session_log_handler_unittest.cc
@@ -227,12 +227,12 @@
session_log_handler_->SetLogCreatedClosureForTest(run_loop.QuitClosure());
web_ui_.HandleReceivedMessage("saveSessionLog", &args);
run_loop.Run();
- const std::string expected_telemetry_log_header = "=== Telemetry Log ===";
+ const std::string expected_system_log_header = "=== System ===";
const std::string expected_system_info_section_name = "--- System Info ---";
const std::string expected_snapshot_time_prefix = "Snapshot Time: ";
const std::vector<std::string> log_lines = GetCombinedLogContents(log_path);
ASSERT_EQ(13u, log_lines.size());
- EXPECT_EQ(expected_telemetry_log_header, log_lines[0]);
+ EXPECT_EQ(expected_system_log_header, log_lines[0]);
EXPECT_EQ(expected_system_info_section_name, log_lines[1]);
EXPECT_GT(log_lines[2].size(), expected_snapshot_time_prefix.size());
EXPECT_TRUE(base::StartsWith(log_lines[2], expected_snapshot_time_prefix));
@@ -251,7 +251,7 @@
EXPECT_EQ("Version: " + expected_full_version, log_lines[9]);
EXPECT_EQ("Has Battery: true", log_lines[10]);
- const std::string expected_routine_log_header = "=== Routine Log ===";
+ const std::string expected_routine_log_header = "--- Test Routines ---";
EXPECT_EQ(expected_routine_log_header, log_lines[11]);
const std::vector<std::string> first_routine_log_line_contents =
diff --git a/ash/webui/diagnostics_ui/backend/system_routine_controller_unittest.cc b/ash/webui/diagnostics_ui/backend/system_routine_controller_unittest.cc
index 2e6e2db..557c6ede 100644
--- a/ash/webui/diagnostics_ui/backend/system_routine_controller_unittest.cc
+++ b/ash/webui/diagnostics_ui/backend/system_routine_controller_unittest.cc
@@ -736,7 +736,7 @@
// Verify that the Running status appears in the log.
std::vector<std::string> log_lines =
- GetLogLines(log.GetContentsForCategory("all"));
+ GetLogLines(log.GetContentsForCategory("system"));
EXPECT_EQ(1u, log_lines.size());
std::vector<std::string> log_line_contents = GetLogLineContents(log_lines[0]);
@@ -756,7 +756,7 @@
mojom::StandardRoutineResult::kTestPassed);
// Verify that the Passed status appears in the log.
- log_lines = GetLogLines(log.GetContentsForCategory("all"));
+ log_lines = GetLogLines(log.GetContentsForCategory("system"));
EXPECT_EQ(2u, log_lines.size());
log_line_contents = GetLogLineContents(log_lines[1]);
@@ -780,7 +780,7 @@
routine_runner_2.reset();
task_environment_.RunUntilIdle();
- log_lines = GetLogLines(log.GetContentsForCategory("all"));
+ log_lines = GetLogLines(log.GetContentsForCategory("system"));
EXPECT_EQ(4u, log_lines.size());
log_line_contents = GetLogLineContents(log_lines[3]);
