Minor improvements on bugreport generation.
- Skipped artificial 100/100 message, since pulling will take care of the final 100% progress. - Consolidated unsupported lines in just one message. - Let user know the bugreport can still be recovered when it could not be copied to the destination directory. BUG: 30451114 Change-Id: Icfce9c1e8e7ed407719728b9874679ac40b21eab
This commit is contained in:
parent
f85554e12f
commit
80a65d03c9
2 changed files with 60 additions and 36 deletions
|
@ -15,6 +15,7 @@
|
|||
*/
|
||||
|
||||
#include <string>
|
||||
#include <vector>
|
||||
|
||||
#include <android-base/strings.h>
|
||||
|
||||
|
@ -31,10 +32,12 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
|
|||
public:
|
||||
BugreportStandardStreamsCallback(const std::string& dest_file, bool show_progress, Bugreport* br)
|
||||
: br_(br),
|
||||
src_file_(),
|
||||
dest_file_(dest_file),
|
||||
line_message_(android::base::StringPrintf("generating %s", dest_file_.c_str())),
|
||||
invalid_lines_(),
|
||||
show_progress_(show_progress),
|
||||
status_(-1),
|
||||
status_(0),
|
||||
line_() {
|
||||
}
|
||||
|
||||
|
@ -54,9 +57,39 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
|
|||
OnStream(nullptr, stderr, buffer, length);
|
||||
}
|
||||
int Done(int unused_) {
|
||||
// Process remaining line, if any...
|
||||
// Process remaining line, if any.
|
||||
ProcessLine(line_);
|
||||
// ..then return.
|
||||
|
||||
// Warn about invalid lines, if any.
|
||||
if (!invalid_lines_.empty()) {
|
||||
fprintf(stderr,
|
||||
"WARNING: bugreportz generated %zu line(s) with unknown commands, "
|
||||
"device might not support zipped bugreports:\n",
|
||||
invalid_lines_.size());
|
||||
for (const auto& line : invalid_lines_) {
|
||||
fprintf(stderr, "\t%s\n", line.c_str());
|
||||
}
|
||||
fprintf(stderr,
|
||||
"If the zipped bugreport was not generated, try 'adb bugreport' instead.\n");
|
||||
}
|
||||
|
||||
// Pull the generated bug report.
|
||||
if (status_ == 0) {
|
||||
if (src_file_.empty()) {
|
||||
fprintf(stderr, "bugreportz did not return a '%s' or '%s' line\n", BUGZ_OK_PREFIX,
|
||||
BUGZ_FAIL_PREFIX);
|
||||
return -1;
|
||||
}
|
||||
std::vector<const char*> srcs{src_file_.c_str()};
|
||||
status_ = br_->DoSyncPull(srcs, dest_file_.c_str(), true, line_message_.c_str()) ? 0 : 1;
|
||||
if (status_ != 0) {
|
||||
fprintf(stderr,
|
||||
"Bug report finished but could not be copied to '%s'.\n"
|
||||
"Try to run 'adb pull %s <directory>'\n"
|
||||
"to copy it to a directory that can be written.\n",
|
||||
dest_file_.c_str(), src_file_.c_str());
|
||||
}
|
||||
}
|
||||
return status_;
|
||||
}
|
||||
|
||||
|
@ -65,17 +98,7 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
|
|||
if (line.empty()) return;
|
||||
|
||||
if (android::base::StartsWith(line, BUGZ_OK_PREFIX)) {
|
||||
if (show_progress_) {
|
||||
// Make sure pull message doesn't conflict with generation message.
|
||||
br_->UpdateProgress(line_message_, 100, 100);
|
||||
}
|
||||
|
||||
const char* zip_file = &line[strlen(BUGZ_OK_PREFIX)];
|
||||
std::vector<const char*> srcs{zip_file};
|
||||
status_ = br_->DoSyncPull(srcs, dest_file_.c_str(), true, line_message_.c_str()) ? 0 : 1;
|
||||
if (status_ != 0) {
|
||||
fprintf(stderr, "Could not copy file '%s' to '%s'\n", zip_file, dest_file_.c_str());
|
||||
}
|
||||
src_file_ = &line[strlen(BUGZ_OK_PREFIX)];
|
||||
} else if (android::base::StartsWith(line, BUGZ_FAIL_PREFIX)) {
|
||||
const char* error_message = &line[strlen(BUGZ_FAIL_PREFIX)];
|
||||
fprintf(stderr, "Device failed to take a zipped bugreport: %s\n", error_message);
|
||||
|
@ -91,17 +114,15 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
|
|||
int total = std::stoi(line.substr(idx2 + 1));
|
||||
br_->UpdateProgress(dest_file_, progress, total);
|
||||
} else {
|
||||
fprintf(stderr,
|
||||
"WARNING: unexpected line (%s) returned by bugreportz, "
|
||||
"device probably does not support zipped bugreports.\n"
|
||||
"Try 'adb bugreport' instead.",
|
||||
line.c_str());
|
||||
invalid_lines_.push_back(line);
|
||||
}
|
||||
}
|
||||
|
||||
Bugreport* br_;
|
||||
std::string src_file_;
|
||||
const std::string dest_file_;
|
||||
const std::string line_message_;
|
||||
std::vector<std::string> invalid_lines_;
|
||||
bool show_progress_;
|
||||
int status_;
|
||||
|
||||
|
@ -132,19 +153,16 @@ int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc,
|
|||
std::string bugz_stderr;
|
||||
DefaultStandardStreamsCallback version_callback(nullptr, &bugz_stderr);
|
||||
int status = SendShellCommand(transport_type, serial, "bugreportz -v", false, &version_callback);
|
||||
|
||||
if (status != 0) {
|
||||
fprintf(stderr,
|
||||
"Failed to get bugreportz version: 'bugreport -v' returned '%s' "
|
||||
"(code %d)."
|
||||
"\nIf the device does not support it, try running 'adb bugreport' "
|
||||
"to get a "
|
||||
"flat-file bugreport.",
|
||||
bugz_stderr.c_str(), status);
|
||||
return status;
|
||||
}
|
||||
std::string bugz_version = android::base::Trim(bugz_stderr);
|
||||
|
||||
if (status != 0 || bugz_version.empty()) {
|
||||
fprintf(stderr,
|
||||
"Failed to get bugreportz version: 'bugreportz -v' returned '%s' (code %d).\n"
|
||||
"If the device runs Android M or below, try 'adb bugreport' instead.\n",
|
||||
bugz_stderr.c_str(), status);
|
||||
return status != 0 ? status : -1;
|
||||
}
|
||||
|
||||
bool show_progress = true;
|
||||
std::string bugz_command = "bugreportz -p";
|
||||
if (bugz_version == "1.0") {
|
||||
|
@ -153,8 +171,7 @@ int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc,
|
|||
fprintf(stderr,
|
||||
"Bugreport is in progress and it could take minutes to complete.\n"
|
||||
"Please be patient and do not cancel or disconnect your device "
|
||||
"until it completes."
|
||||
"\n");
|
||||
"until it completes.\n");
|
||||
show_progress = false;
|
||||
bugz_command = "bugreportz";
|
||||
}
|
||||
|
|
|
@ -189,14 +189,14 @@ TEST_F(BugreportTest, Ok) {
|
|||
ExpectProgress(10, 100);
|
||||
ExpectProgress(50, 100);
|
||||
ExpectProgress(99, 100);
|
||||
ExpectProgress(100, 100);
|
||||
// clang-format off
|
||||
EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
|
||||
// NOTE: DoAll accepts at most 10 arguments, and we have reached that limit...
|
||||
.WillOnce(DoAll(
|
||||
// Progress line in one write
|
||||
WithArg<4>(WriteOnStdout("PROGRESS:1/100\n")),
|
||||
// Add some bogus lines
|
||||
WithArg<4>(WriteOnStdout("\nDUDE:SWEET\n\n")),
|
||||
WithArg<4>(WriteOnStdout("\nDUDE:SWEET\n\nBLA\n\nBLA\nBLA\n\n")),
|
||||
// Multiple progress lines in one write
|
||||
WithArg<4>(WriteOnStdout("PROGRESS:10/100\nPROGRESS:50/100\n")),
|
||||
// Progress line in multiple writes
|
||||
|
@ -207,6 +207,7 @@ TEST_F(BugreportTest, Ok) {
|
|||
WithArg<4>(WriteOnStdout("OK:/device/bugreport")),
|
||||
WithArg<4>(WriteOnStdout(".zip")),
|
||||
WithArg<4>(ReturnCallbackDone())));
|
||||
// clang-format on
|
||||
EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"),
|
||||
true, HasSubstr("file.zip")))
|
||||
.WillOnce(Return(true));
|
||||
|
@ -218,7 +219,6 @@ TEST_F(BugreportTest, Ok) {
|
|||
// Tests 'adb bugreport file' when it succeeds
|
||||
TEST_F(BugreportTest, OkNoExtension) {
|
||||
SetBugreportzVersion("1.1");
|
||||
ExpectProgress(100, 100);
|
||||
EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
|
||||
.WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip\n")),
|
||||
WithArg<4>(ReturnCallbackDone())));
|
||||
|
@ -281,6 +281,14 @@ TEST_F(BugreportTest, BugreportzVersionFailed) {
|
|||
ASSERT_EQ(666, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
|
||||
}
|
||||
|
||||
// Tests 'adb bugreport file.zip' when the bugreportz -v returns status 0 but with no output.
|
||||
TEST_F(BugreportTest, BugreportzVersionEmpty) {
|
||||
SetBugreportzVersion("");
|
||||
|
||||
const char* args[1024] = {"bugreport", "file.zip"};
|
||||
ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
|
||||
}
|
||||
|
||||
// Tests 'adb bugreport file.zip' when the main bugreportz command failed
|
||||
TEST_F(BugreportTest, BugreportzFailed) {
|
||||
SetBugreportzVersion("1.1");
|
||||
|
@ -294,7 +302,6 @@ TEST_F(BugreportTest, BugreportzFailed) {
|
|||
// Tests 'adb bugreport file.zip' when the bugreport could not be pulled
|
||||
TEST_F(BugreportTest, PullFails) {
|
||||
SetBugreportzVersion("1.1");
|
||||
ExpectProgress(100, 100);
|
||||
EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
|
||||
.WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip")),
|
||||
WithArg<4>(ReturnCallbackDone())));
|
||||
|
|
Loading…
Reference in a new issue