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:
Felipe Leme 2016-07-29 15:47:00 -07:00
parent f85554e12f
commit 80a65d03c9
2 changed files with 60 additions and 36 deletions

View file

@ -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";
}

View file

@ -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())));