Minor improvements on bugreport generation. am: 80a65d03c9 am: 7939117c98 (c925b83c) · Commits · e / os / android_system_core

adb/bugreport.cpp

+46 −29

Original line number	Original line	Diff line number	Diff line
	@@ -15,6 +15,7 @@
	*/		*/

	#include <string>		#include <string>
			#include <vector>

	#include <android-base/strings.h>		#include <android-base/strings.h>

	@@ -31,10 +32,12 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
	public:		public:
	BugreportStandardStreamsCallback(const std::string& dest_file, bool show_progress, Bugreport* br)		BugreportStandardStreamsCallback(const std::string& dest_file, bool show_progress, Bugreport* br)
	: br_(br),		: br_(br),
			src_file_(),
	dest_file_(dest_file),		dest_file_(dest_file),
	line_message_(android::base::StringPrintf("generating %s", dest_file_.c_str())),		line_message_(android::base::StringPrintf("generating %s", dest_file_.c_str())),
			invalid_lines_(),
	show_progress_(show_progress),		show_progress_(show_progress),
	status_(-1),		status_(0),
	line_() {		line_() {
	}		}

	@@ -54,9 +57,39 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
	OnStream(nullptr, stderr, buffer, length);		OnStream(nullptr, stderr, buffer, length);
	}		}
	int Done(int unused_) {		int Done(int unused_) {
	// Process remaining line, if any...		// Process remaining line, if any.
	ProcessLine(line_);		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_;		return status_;
	}		}

	@@ -65,17 +98,7 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
	if (line.empty()) return;		if (line.empty()) return;

	if (android::base::StartsWith(line, BUGZ_OK_PREFIX)) {		if (android::base::StartsWith(line, BUGZ_OK_PREFIX)) {
	if (show_progress_) {		src_file_ = &line[strlen(BUGZ_OK_PREFIX)];
	// 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());
	}
	} else if (android::base::StartsWith(line, BUGZ_FAIL_PREFIX)) {		} else if (android::base::StartsWith(line, BUGZ_FAIL_PREFIX)) {
	const char* error_message = &line[strlen(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);		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));		int total = std::stoi(line.substr(idx2 + 1));
	br_->UpdateProgress(dest_file_, progress, total);		br_->UpdateProgress(dest_file_, progress, total);
	} else {		} else {
	fprintf(stderr,		invalid_lines_.push_back(line);
	"WARNING: unexpected line (%s) returned by bugreportz, "
	"device probably does not support zipped bugreports.\n"
	"Try 'adb bugreport' instead.",
	line.c_str());
	}		}
	}		}

	Bugreport* br_;		Bugreport* br_;
			std::string src_file_;
	const std::string dest_file_;		const std::string dest_file_;
	const std::string line_message_;		const std::string line_message_;
			std::vector<std::string> invalid_lines_;
	bool show_progress_;		bool show_progress_;
	int status_;		int status_;

	@@ -132,18 +153,15 @@ int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc,
	std::string bugz_stderr;		std::string bugz_stderr;
	DefaultStandardStreamsCallback version_callback(nullptr, &bugz_stderr);		DefaultStandardStreamsCallback version_callback(nullptr, &bugz_stderr);
	int status = SendShellCommand(transport_type, serial, "bugreportz -v", false, &version_callback);		int status = SendShellCommand(transport_type, serial, "bugreportz -v", false, &version_callback);
			std::string bugz_version = android::base::Trim(bugz_stderr);

	if (status != 0) {		if (status != 0 \|\| bugz_version.empty()) {
	fprintf(stderr,		fprintf(stderr,
	"Failed to get bugreportz version: 'bugreport -v' returned '%s' "		"Failed to get bugreportz version: 'bugreportz -v' returned '%s' (code %d).\n"
	"(code %d)."		"If the device runs Android M or below, try 'adb bugreport' instead.\n",
	"\nIf the device does not support it, try running 'adb bugreport' "
	"to get a "
	"flat-file bugreport.",
	bugz_stderr.c_str(), status);		bugz_stderr.c_str(), status);
	return status;		return status != 0 ? status : -1;
	}		}
	std::string bugz_version = android::base::Trim(bugz_stderr);

	bool show_progress = true;		bool show_progress = true;
	std::string bugz_command = "bugreportz -p";		std::string bugz_command = "bugreportz -p";
	@@ -153,8 +171,7 @@ int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc,
	fprintf(stderr,		fprintf(stderr,
	"Bugreport is in progress and it could take minutes to complete.\n"		"Bugreport is in progress and it could take minutes to complete.\n"
	"Please be patient and do not cancel or disconnect your device "		"Please be patient and do not cancel or disconnect your device "
	"until it completes."		"until it completes.\n");
	"\n");
	show_progress = false;		show_progress = false;
	bugz_command = "bugreportz";		bugz_command = "bugreportz";
	}		}

adb/bugreport_test.cpp

+11 −4

Original line number	Original line	Diff line number	Diff line
	@@ -189,14 +189,14 @@ TEST_F(BugreportTest, Ok) {
	ExpectProgress(10, 100);		ExpectProgress(10, 100);
	ExpectProgress(50, 100);		ExpectProgress(50, 100);
	ExpectProgress(99, 100);		ExpectProgress(99, 100);
	ExpectProgress(100, 100);
	// clang-format off		// clang-format off
	EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))		EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
			// NOTE: DoAll accepts at most 10 arguments, and we have reached that limit...
	.WillOnce(DoAll(		.WillOnce(DoAll(
	// Progress line in one write		// Progress line in one write
	WithArg<4>(WriteOnStdout("PROGRESS:1/100\n")),		WithArg<4>(WriteOnStdout("PROGRESS:1/100\n")),
	// Add some bogus lines		// 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		// Multiple progress lines in one write
	WithArg<4>(WriteOnStdout("PROGRESS:10/100\nPROGRESS:50/100\n")),		WithArg<4>(WriteOnStdout("PROGRESS:10/100\nPROGRESS:50/100\n")),
	// Progress line in multiple writes		// Progress line in multiple writes
	@@ -207,6 +207,7 @@ TEST_F(BugreportTest, Ok) {
	WithArg<4>(WriteOnStdout("OK:/device/bugreport")),		WithArg<4>(WriteOnStdout("OK:/device/bugreport")),
	WithArg<4>(WriteOnStdout(".zip")),		WithArg<4>(WriteOnStdout(".zip")),
	WithArg<4>(ReturnCallbackDone())));		WithArg<4>(ReturnCallbackDone())));
			// clang-format on
	EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"),		EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"),
	true, HasSubstr("file.zip")))		true, HasSubstr("file.zip")))
	.WillOnce(Return(true));		.WillOnce(Return(true));
	@@ -218,7 +219,6 @@ TEST_F(BugreportTest, Ok) {
	// Tests 'adb bugreport file' when it succeeds		// Tests 'adb bugreport file' when it succeeds
	TEST_F(BugreportTest, OkNoExtension) {		TEST_F(BugreportTest, OkNoExtension) {
	SetBugreportzVersion("1.1");		SetBugreportzVersion("1.1");
	ExpectProgress(100, 100);
	EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))		EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
	.WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip\n")),		.WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip\n")),
	WithArg<4>(ReturnCallbackDone())));		WithArg<4>(ReturnCallbackDone())));
	@@ -281,6 +281,14 @@ TEST_F(BugreportTest, BugreportzVersionFailed) {
	ASSERT_EQ(666, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));		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		// Tests 'adb bugreport file.zip' when the main bugreportz command failed
	TEST_F(BugreportTest, BugreportzFailed) {		TEST_F(BugreportTest, BugreportzFailed) {
	SetBugreportzVersion("1.1");		SetBugreportzVersion("1.1");
	@@ -294,7 +302,6 @@ TEST_F(BugreportTest, BugreportzFailed) {
	// Tests 'adb bugreport file.zip' when the bugreport could not be pulled		// Tests 'adb bugreport file.zip' when the bugreport could not be pulled
	TEST_F(BugreportTest, PullFails) {		TEST_F(BugreportTest, PullFails) {
	SetBugreportzVersion("1.1");		SetBugreportzVersion("1.1");
	ExpectProgress(100, 100);
	EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))		EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
	.WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip")),		.WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip")),
	WithArg<4>(ReturnCallbackDone())));		WithArg<4>(ReturnCallbackDone())));