Don't display bugreport progress when it recedes. (dedcbaad) · Commits · e / os / android_system_core

adb/bugreport.cpp

+12 −1

Original line number	Original line	Diff line number	Diff line
	@@ -47,7 +47,8 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
	invalid_lines_(),		invalid_lines_(),
	show_progress_(show_progress),		show_progress_(show_progress),
	status_(0),		status_(0),
	line_() {		line_(),
			last_progress_(0) {
	SetLineMessage("generating");		SetLineMessage("generating");
	}		}

	@@ -66,6 +67,7 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
	void OnStderr(const char* buffer, int length) {		void OnStderr(const char* buffer, int length) {
	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_);
	@@ -145,6 +147,11 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
	size_t idx1 = line.rfind(BUGZ_PROGRESS_PREFIX) + strlen(BUGZ_PROGRESS_PREFIX);		size_t idx1 = line.rfind(BUGZ_PROGRESS_PREFIX) + strlen(BUGZ_PROGRESS_PREFIX);
	size_t idx2 = line.rfind(BUGZ_PROGRESS_SEPARATOR);		size_t idx2 = line.rfind(BUGZ_PROGRESS_SEPARATOR);
	int progress = std::stoi(line.substr(idx1, (idx2 - idx1)));		int progress = std::stoi(line.substr(idx1, (idx2 - idx1)));
			if (progress <= last_progress_) {
			// Ignore.
			return;
			}
			last_progress_ = progress;
	int total = std::stoi(line.substr(idx2 + 1));		int total = std::stoi(line.substr(idx2 + 1));
	br_->UpdateProgress(line_message_, progress, total);		br_->UpdateProgress(line_message_, progress, total);
	} else {		} else {
	@@ -180,6 +187,10 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
	// Temporary buffer containing the characters read since the last newline (\n).		// Temporary buffer containing the characters read since the last newline (\n).
	std::string line_;		std::string line_;

			// Last displayed progress.
			// Since dumpstate progress can recede, only forward progress should be displayed
			int last_progress_;

	DISALLOW_COPY_AND_ASSIGN(BugreportStandardStreamsCallback);		DISALLOW_COPY_AND_ASSIGN(BugreportStandardStreamsCallback);
	};		};

adb/bugreport_test.cpp

+31 −4

Original line number	Original line	Diff line number	Diff line
	@@ -50,9 +50,7 @@ bool do_sync_pull(const std::vector<const char>& srcs, const char dst, bool co

	// Empty functions so tests don't need to be linked against commandline.cpp		// Empty functions so tests don't need to be linked against commandline.cpp
	DefaultStandardStreamsCallback DEFAULT_STANDARD_STREAMS_CALLBACK(nullptr, nullptr);		DefaultStandardStreamsCallback DEFAULT_STANDARD_STREAMS_CALLBACK(nullptr, nullptr);
	int usage() {
	return -42;
	}
	int send_shell_command(TransportType transport_type, const char* serial, const std::string& command,		int send_shell_command(TransportType transport_type, const char* serial, const std::string& command,
	bool disable_shell_protocol, StandardStreamsCallbackInterface* callback) {		bool disable_shell_protocol, StandardStreamsCallbackInterface* callback) {
	ADD_FAILURE() << "send_shell_command() should have been mocked";		ADD_FAILURE() << "send_shell_command() should have been mocked";
	@@ -155,7 +153,7 @@ class BugreportTest : public ::testing::Test {
	// Tests when called with invalid number of arguments		// Tests when called with invalid number of arguments
	TEST_F(BugreportTest, InvalidNumberArgs) {		TEST_F(BugreportTest, InvalidNumberArgs) {
	const char* args[] = {"bugreport", "to", "principal"};		const char* args[] = {"bugreport", "to", "principal"};
	ASSERT_EQ(-42, br_.DoIt(kTransportLocal, "HannibalLecter", 3, args));		ASSERT_EQ(1, br_.DoIt(kTransportLocal, "HannibalLecter", 3, args));
	}		}

	// Tests the 'adb bugreport' option when the device does not support 'bugreportz' - it falls back		// Tests the 'adb bugreport' option when the device does not support 'bugreportz' - it falls back
	@@ -282,6 +280,35 @@ TEST_F(BugreportTest, OkProgress) {
	ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));		ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
	}		}

			// Tests 'adb bugreport file.zip' when it succeeds and displays progress, even if progress recedes.
			TEST_F(BugreportTest, OkProgressAlwaysForward) {
			ExpectBugreportzVersion("1.1");
			ExpectProgress(1, 100);
			ExpectProgress(50, 100);
			ExpectProgress(75, 100);
			// clang-format off
			EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
			// NOTE: DoAll accepts at most 10 arguments, and we're almost reached that limit...
			.WillOnce(DoAll(
			WithArg<4>(WriteOnStdout("BEGIN:/device/bugreport.zip\n")),
			WithArg<4>(WriteOnStdout("PROGRESS:1/100\n")),
			WithArg<4>(WriteOnStdout("PROGRESS:50/100\n")),
			// 25 should be ignored becaused it receded.
			WithArg<4>(WriteOnStdout("PROGRESS:25/100\n")),
			WithArg<4>(WriteOnStdout("PROGRESS:75/100\n")),
			// 75 should be ignored becaused it didn't change.
			WithArg<4>(WriteOnStdout("PROGRESS:75/100\n")),
			WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip")),
			WithArg<4>(ReturnCallbackDone())));
			// clang-format on
			EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"),
			true, StrEq("pulling file.zip")))
			.WillOnce(Return(true));

			const char* args[] = {"bugreport", "file.zip"};
			ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
			}

	// Tests 'adb bugreport dir' when it succeeds and destination is a directory.		// Tests 'adb bugreport dir' when it succeeds and destination is a directory.
	TEST_F(BugreportTest, OkDirectory) {		TEST_F(BugreportTest, OkDirectory) {
	ExpectBugreportzVersion("1.1");		ExpectBugreportzVersion("1.1");