Don't display bugreport progress when it recedes, for real...
The previous fix was taking account just the progress reported by dumpstate,
not progress/percentage. As such, it was not detecting the cases where the
percentage decreased but the progress didn't.
Bug: 37878670
Test: m -j32 adb_test && ./out/host/linux-x86/nativetest64/adb_test/adb_test --gtest_filter=BugreportTest.*
Change-Id: I5830028f3191a9b17f63aeed5c049b29fa7d1179
(cherry picked from commit 4cc03611cd
)
This commit is contained in:
parent
39225a131b
commit
53fd1730b6
3 changed files with 29 additions and 27 deletions
|
@ -48,7 +48,7 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
|
|||
show_progress_(show_progress),
|
||||
status_(0),
|
||||
line_(),
|
||||
last_progress_(0) {
|
||||
last_progress_percentage_(0) {
|
||||
SetLineMessage("generating");
|
||||
}
|
||||
|
||||
|
@ -147,13 +147,14 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
|
|||
size_t idx1 = line.rfind(BUGZ_PROGRESS_PREFIX) + strlen(BUGZ_PROGRESS_PREFIX);
|
||||
size_t idx2 = line.rfind(BUGZ_PROGRESS_SEPARATOR);
|
||||
int progress = std::stoi(line.substr(idx1, (idx2 - idx1)));
|
||||
if (progress <= last_progress_) {
|
||||
int total = std::stoi(line.substr(idx2 + 1));
|
||||
int progress_percentage = (progress * 100 / total);
|
||||
if (progress_percentage <= last_progress_percentage_) {
|
||||
// Ignore.
|
||||
return;
|
||||
}
|
||||
last_progress_ = progress;
|
||||
int total = std::stoi(line.substr(idx2 + 1));
|
||||
br_->UpdateProgress(line_message_, progress, total);
|
||||
last_progress_percentage_ = progress_percentage;
|
||||
br_->UpdateProgress(line_message_, progress_percentage);
|
||||
} else {
|
||||
invalid_lines_.push_back(line);
|
||||
}
|
||||
|
@ -189,7 +190,7 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface
|
|||
|
||||
// Last displayed progress.
|
||||
// Since dumpstate progress can recede, only forward progress should be displayed
|
||||
int last_progress_;
|
||||
int last_progress_percentage_;
|
||||
|
||||
DISALLOW_COPY_AND_ASSIGN(BugreportStandardStreamsCallback);
|
||||
};
|
||||
|
@ -267,8 +268,7 @@ int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc,
|
|||
return SendShellCommand(transport_type, serial, bugz_command, false, &bugz_callback);
|
||||
}
|
||||
|
||||
void Bugreport::UpdateProgress(const std::string& message, int progress, int total) {
|
||||
int progress_percentage = (progress * 100 / total);
|
||||
void Bugreport::UpdateProgress(const std::string& message, int progress_percentage) {
|
||||
line_printer_.Print(
|
||||
android::base::StringPrintf("[%3d%%] %s", progress_percentage, message.c_str()),
|
||||
LinePrinter::INFO);
|
||||
|
|
|
@ -43,7 +43,7 @@ class Bugreport {
|
|||
const char* name);
|
||||
|
||||
private:
|
||||
virtual void UpdateProgress(const std::string& file_name, int progress, int total);
|
||||
virtual void UpdateProgress(const std::string& file_name, int progress_percentage);
|
||||
LinePrinter line_printer_;
|
||||
DISALLOW_COPY_AND_ASSIGN(Bugreport);
|
||||
};
|
||||
|
|
|
@ -123,7 +123,7 @@ class BugreportMock : public Bugreport {
|
|||
bool disable_shell_protocol, StandardStreamsCallbackInterface* callback));
|
||||
MOCK_METHOD4(DoSyncPull, bool(const std::vector<const char*>& srcs, const char* dst,
|
||||
bool copy_attrs, const char* name));
|
||||
MOCK_METHOD3(UpdateProgress, void(const std::string&, int, int));
|
||||
MOCK_METHOD2(UpdateProgress, void(const std::string&, int));
|
||||
};
|
||||
|
||||
class BugreportTest : public ::testing::Test {
|
||||
|
@ -142,8 +142,8 @@ class BugreportTest : public ::testing::Test {
|
|||
WithArg<4>(ReturnCallbackDone(0))));
|
||||
}
|
||||
|
||||
void ExpectProgress(int progress, int total, const std::string& file = "file.zip") {
|
||||
EXPECT_CALL(br_, UpdateProgress(StrEq("generating " + file), progress, total));
|
||||
void ExpectProgress(int progress_percentage, const std::string& file = "file.zip") {
|
||||
EXPECT_CALL(br_, UpdateProgress(StrEq("generating " + file), progress_percentage));
|
||||
}
|
||||
|
||||
BugreportMock br_;
|
||||
|
@ -200,7 +200,7 @@ TEST_F(BugreportTest, NoArgumentsPostNDevice) {
|
|||
ExpectBugreportzVersion("1.1");
|
||||
std::string dest_file =
|
||||
android::base::StringPrintf("%s%cda_bugreport.zip", cwd_.c_str(), OS_PATH_SEPARATOR);
|
||||
ExpectProgress(50, 100, "da_bugreport.zip");
|
||||
ExpectProgress(50, "da_bugreport.zip");
|
||||
EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
|
||||
.WillOnce(DoAll(WithArg<4>(WriteOnStdout("BEGIN:/device/da_bugreport.zip\n")),
|
||||
WithArg<4>(WriteOnStdout("PROGRESS:50/100\n")),
|
||||
|
@ -247,10 +247,10 @@ TEST_F(BugreportTest, OkNDeviceSplitBuffer) {
|
|||
// Tests 'adb bugreport file.zip' when it succeeds and displays progress.
|
||||
TEST_F(BugreportTest, OkProgress) {
|
||||
ExpectBugreportzVersion("1.1");
|
||||
ExpectProgress(1, 100);
|
||||
ExpectProgress(10, 100);
|
||||
ExpectProgress(50, 100);
|
||||
ExpectProgress(99, 100);
|
||||
ExpectProgress(1);
|
||||
ExpectProgress(10);
|
||||
ExpectProgress(50);
|
||||
ExpectProgress(99);
|
||||
// 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...
|
||||
|
@ -283,21 +283,23 @@ TEST_F(BugreportTest, OkProgress) {
|
|||
// 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);
|
||||
ExpectProgress(1);
|
||||
ExpectProgress(50);
|
||||
ExpectProgress(75);
|
||||
// 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("PROGRESS:1/100\n")), // 1%
|
||||
WithArg<4>(WriteOnStdout("PROGRESS:50/100\n")), // 50%
|
||||
// 25% should be ignored becaused it receded.
|
||||
WithArg<4>(WriteOnStdout("PROGRESS:25/100\n")), // 25%
|
||||
WithArg<4>(WriteOnStdout("PROGRESS:75/100\n")), // 75%
|
||||
// 75% should be ignored becaused it didn't change.
|
||||
WithArg<4>(WriteOnStdout("PROGRESS:75/100\n")), // 75%
|
||||
// Try a receeding percentage with a different max progress
|
||||
WithArg<4>(WriteOnStdout("PROGRESS:700/1000\n")), // 70%
|
||||
WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip")),
|
||||
WithArg<4>(ReturnCallbackDone())));
|
||||
// clang-format on
|
||||
|
|
Loading…
Reference in a new issue