Skip to content

GitLab

T
tangled
Commit aa698d24 authored by Lucía Cantú-Miller's avatar Lucía Cantú-Miller Committed by Commit Bot
Browse files
Options

Remove obsolete code + update unit test 

Several console methods were intended for old Chrome versions that are
not longer supported. Console.messageAdded event was eliminated and
Chrome 54 TODOS where resolved. Unit test was updated to test newer
Chrome versions.

Bug: chromedriver:3518
Change-Id: Iebf08a32a3c90d3ffd200fb81686c54a636d49f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2259132Reviewed-by: default avatarJohn Chen <johnchen@chromium.org>
Commit-Queue: Lucía Cantú-Miller <cantumiller@google.com>
Cr-Commit-Position: refs/heads/master@{#782034}
parent 807af178
Show whitespace changes
Inline Side-by-side
Showing with 32 additions and 131 deletions
chrome/test/chromedriver/chrome/console_logger.cc
View file @ aa698d24
......@@ -42,17 +42,8 @@ Status ConsoleLogger::OnConnected(DevToolsClient* client) {
base::DictionaryValue params;
Status status = client->SendCommand("Log.enable", params);
if (status.IsError()) {
std::string message = status.message();
if (message.find("'Log.enable' wasn't found") != std::string::npos) {
// If the Log.enable command doesn't exist, then we're on Chrome 53 or
// earlier. Enable the Console domain so we can listen for
// Console.messageAdded events.
return client->SendCommand("Console.enable", params);
}
return status;
}
// Otherwise, we're on Chrome 54+. Enable the Log and Runtime domains so we
// can listen for Log.entryAdded and Runtime.exceptionThrown events.
return client->SendCommand("Runtime.enable", params);
}
......@@ -60,8 +51,6 @@ Status ConsoleLogger::OnEvent(
DevToolsClient* client,
const std::string& method,
const base::DictionaryValue& params) {
if (method == "Console.messageAdded")
return OnConsoleMessageAdded(params);
if (method == "Log.entryAdded")
return OnLogEntryAdded(params);
if (method == "Runtime.consoleAPICalled")
......@@ -71,58 +60,6 @@ Status ConsoleLogger::OnEvent(
return Status(kOk);
}
Status ConsoleLogger::OnConsoleMessageAdded(
const base::DictionaryValue& params) {
// If the event has proper structure and fields, log formatted.
// Else it's a weird message that we don't know how to format, log full JSON.
const base::DictionaryValue* message_dict = nullptr;
if (params.GetDictionary("message", &message_dict)) {
std::string text;
std::string level_name;
Log::Level level = Log::kInfo;
if (message_dict->GetString("text", &text) && !text.empty() &&
message_dict->GetString("level", &level_name) &&
ConsoleLevelToLogLevel(level_name, &level)) {
const char* origin_cstr = "unknown";
std::string origin;
if ((message_dict->GetString("url", &origin) && !origin.empty()) ||
(message_dict->GetString("source", &origin) && !origin.empty())) {
origin_cstr = origin.c_str();
}
std::string line_column;
int line = -1;
if (message_dict->GetInteger("line", &line)) {
int column = -1;
if (message_dict->GetInteger("column", &column)) {
base::SStringPrintf(&line_column, "%d:%d", line, column);
} else {
base::SStringPrintf(&line_column, "%d", line);
}
} else {
// No line number, but print anyway, just to maintain the number of
// fields in the formatted message in case someone wants to parse it.
line_column = "-";
}
std::string source;
message_dict->GetString("source", &source);
log_->AddEntry(level, source, base::StringPrintf("%s %s %s",
origin_cstr,
line_column.c_str(),
text.c_str()));
return Status(kOk);
}
}
// Don't know how to format, log full JSON.
std::string message_json;
base::JSONWriter::Write(params, &message_json);
log_->AddEntry(Log::kWarning, message_json);
return Status(kOk);
}
Status ConsoleLogger::OnLogEntryAdded(const base::DictionaryValue& params) {
const base::DictionaryValue* entry = nullptr;
if (!params.GetDictionary("entry", &entry))
......@@ -240,12 +177,8 @@ Status ConsoleLogger::OnRuntimeConsoleApiCalled(
Status ConsoleLogger::OnRuntimeExceptionThrown(
const base::DictionaryValue& params) {
const base::DictionaryValue* exception_details = nullptr;
// In Chrome 54, |url|, |lineNumber| and |columnNumber| are properties of the
// |details| dictionary. In Chrome 55+, they are inside the |exceptionDetails|
// dictionary.
// TODO(samuong): Stop looking at |details| once we stop supporting Chrome 54.
if (!params.GetDictionary("exceptionDetails", &exception_details))
if (!params.GetDictionary("details", &exception_details))
return Status(kUnknownError, "missing or invalid exception details");
std::string origin;
......@@ -260,13 +193,6 @@ Status ConsoleLogger::OnRuntimeExceptionThrown(
return Status(kUnknownError, "missing or invalid columnNumber");
std::string line_column = base::StringPrintf("%d:%d", line, column);
// In Chrome 54, the exception object is serialized as a dictionary called
// |exception|. In Chrome 55+, the exception object properties are in the
// |exceptionDetails| object.
// TODO(samuong): Delete this once we stop supporting Chrome 54.
if (!params.GetDictionary("exceptionDetails", &exception_details))
exception_details = &params;
std::string text;
const base::DictionaryValue* exception = nullptr;
const base::DictionaryValue* preview = nullptr;
......
chrome/test/chromedriver/chrome/console_logger.h
View file @ aa698d24
......@@ -33,7 +33,6 @@ class ConsoleLogger : public DevToolsEventListener {
private:
Log* log_; // The log where to create entries.
Status OnConsoleMessageAdded(const base::DictionaryValue& params);
Status OnLogEntryAdded(const base::DictionaryValue& params);
Status OnRuntimeConsoleApiCalled(const base::DictionaryValue& params);
Status OnRuntimeExceptionThrown(const base::DictionaryValue& params);
......
chrome/test/chromedriver/chrome/console_logger_unittest.cc
View file @ aa698d24
......@@ -47,13 +47,6 @@ class FakeDevToolsClient : public StubDevToolsClient {
const std::string& method,
const base::DictionaryValue& params,
std::unique_ptr<base::DictionaryValue>* result) override {
if (method == "Log.enable") {
// FakeDevToolsClient emulates Chrome 53 and earlier versions, which do
// not implement the Log.enable command. ConsoleLogger functionality for
// Chrome 54+ is tested by end-to-end tests in run_py_tests.py.
return Status(kUnknownError, "unhandled inspector error: "
"{\"code\":-32601,\"message\":\"'Log.enable' wasn't found\"}");
}
sent_command_queue_.push(method);
return Status(kOk);
}
......@@ -127,21 +120,18 @@ void ConsoleLogParams(base::DictionaryValue* out_params,
const char* source,
const char* url,
const char* level,
int line,
int column,
int lineNumber,
const char* text) {
if (source)
out_params->SetString("message.source", source);
out_params->SetString("entry.source", source);
if (url)
out_params->SetString("message.url", url);
out_params->SetString("entry.url", url);
if (level)
out_params->SetString("message.level", level);
if (line != -1)
out_params->SetInteger("message.line", line);
if (column != -1)
out_params->SetInteger("message.column", column);
out_params->SetString("entry.level", level);
if (lineNumber != -1)
out_params->SetInteger("entry.lineNumber", lineNumber);
if (text)
out_params->SetString("message.text", text);
out_params->SetString("entry.text", text);
}
} // namespace
......@@ -153,64 +143,50 @@ TEST(ConsoleLogger, ConsoleMessages) {
client.AddListener(&logger);
logger.OnConnected(&client);
EXPECT_EQ("Console.enable", client.PopSentCommand());
EXPECT_EQ("Log.enable", client.PopSentCommand());
EXPECT_EQ("Runtime.enable", client.PopSentCommand());
EXPECT_TRUE(client.PopSentCommand().empty());
base::DictionaryValue params1; // All fields are set.
ConsoleLogParams(&params1, "source1", "url1", "verbose", 10, 1, "text1");
ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params1).code());
ConsoleLogParams(&params1, "source1", "url1", "verbose", 10, "text1");
ASSERT_EQ(kOk, client.TriggerEvent("Log.entryAdded", params1).code());
// Ignored -- wrong method.
ASSERT_EQ(kOk, client.TriggerEvent("Console.gaga", params1).code());
ASSERT_EQ(kOk, client.TriggerEvent("Log.gaga", params1).code());
base::DictionaryValue params2; // All optionals are not set.
ConsoleLogParams(&params2, "source2", nullptr, "log", -1, -1, "text2");
ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params2).code());
base::DictionaryValue params3; // Line without column, no source.
ConsoleLogParams(&params3, nullptr, "url3", "warning", 30, -1, "text3");
ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params3).code());
ConsoleLogParams(&params2, "source2", nullptr, "log", -1, "text2");
ASSERT_EQ(kOk, client.TriggerEvent("Log.entryAdded", params2).code());
base::DictionaryValue params4; // Column without line.
ConsoleLogParams(&params4, "source4", "url4", "error", -1, 4, "text4");
ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params4).code());
base::DictionaryValue params3; // Line, no source.
ConsoleLogParams(&params3, nullptr, "url3", "warning", 30, "text3");
ASSERT_EQ(kUnknownError,
client.TriggerEvent("Log.entryAdded", params3).code());
base::DictionaryValue params5; // Bad level name.
ConsoleLogParams(&params5, "source5", "url5", "gaga", 50, 5, "ulala");
ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params5).code());
ConsoleLogParams(&params5, "source5", "url5", "gaga", 50, "ulala");
ASSERT_EQ(kUnknownError,
client.TriggerEvent("Log.entryAdded", params5).code());
base::DictionaryValue params6; // Unset level.
ConsoleLogParams(&params6, "source6", "url6", nullptr, 60, 6, nullptr);
ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params6).code());
ConsoleLogParams(&params6, "source6", "url6", nullptr, 60, nullptr);
ASSERT_EQ(kUnknownError,
client.TriggerEvent("Log.entryAdded", params6).code());
base::DictionaryValue params7; // No text.
ConsoleLogParams(&params7, "source7", "url7", "log", -1, -1, nullptr);
ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params7).code());
ConsoleLogParams(&params7, "source7", "url7", "log", -1, nullptr);
ASSERT_EQ(kUnknownError,
client.TriggerEvent("Log.entryAdded", params7).code());
base::DictionaryValue params8; // No message object.
params8.SetInteger("gaga", 8);
ASSERT_EQ(kOk, client.TriggerEvent("Console.messageAdded", params8).code());
ASSERT_EQ(kUnknownError,
client.TriggerEvent("Log.entryAdded", params8).code());
EXPECT_TRUE(client.PopSentCommand().empty()); // No other commands sent.
ASSERT_EQ(8u, log.GetEntries().size());
ASSERT_EQ(2u, log.GetEntries().size());
ValidateLogEntry(log.GetEntries()[0].get(), Log::kDebug, "source1",
"url1 10:1 text1");
"url1 10 text1");
ValidateLogEntry(log.GetEntries()[1].get(), Log::kInfo, "source2",
"source2 - text2");
ValidateLogEntry(log.GetEntries()[2].get(), Log::kWarning, "",
"url3 30 text3");
ValidateLogEntry(log.GetEntries()[3].get(), Log::kError, "source4",
"url4 - text4");
ValidateLogEntry(
log.GetEntries()[4].get(), Log::kWarning, "",
"{\"message\":{\"column\":5,\"level\":\"gaga\",\"line\":50,"
"\"source\":\"source5\",\"text\":\"ulala\",\"url\":\"url5\"}}");
ValidateLogEntry(log.GetEntries()[5].get(), Log::kWarning, "",
"{\"message\":{\"column\":6,\"line\":60,"
"\"source\":\"source6\",\"url\":\"url6\"}}");
ValidateLogEntry(log.GetEntries()[6].get(), Log::kWarning, "",
"{\"message\":{\"level\":\"log\","
"\"source\":\"source7\",\"url\":\"url7\"}}");
ValidateLogEntry(log.GetEntries()[7].get(), Log::kWarning, "",
"{\"gaga\":8}");
}
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