diff --git a/adb/bugreport.cpp b/adb/bugreport.cpp index 213fa2e50..9ed44a709 100644 --- a/adb/bugreport.cpp +++ b/adb/bugreport.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#define TRACE_TAG ADB + #include "bugreport.h" #include @@ -187,22 +189,30 @@ int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc, if (argc > 2) return usage(); // Gets bugreportz version. - std::string bugz_stderr; - DefaultStandardStreamsCallback version_callback(nullptr, &bugz_stderr); + std::string bugz_stdout, bugz_stderr; + DefaultStandardStreamsCallback version_callback(&bugz_stdout, &bugz_stderr); int status = SendShellCommand(transport_type, serial, "bugreportz -v", false, &version_callback); std::string bugz_version = android::base::Trim(bugz_stderr); + std::string bugz_output = android::base::Trim(bugz_stdout); if (status != 0 || bugz_version.empty()) { - // Device does not support bugreportz: if called as 'adb bugreport', just falls out to the - // flat-file version - if (argc == 1) return SendShellCommand(transport_type, serial, "bugreport", false); + D("'bugreportz' -v results: status=%d, stdout='%s', stderr='%s'", status, + bugz_output.c_str(), bugz_version.c_str()); + if (argc == 1) { + // Device does not support bugreportz: if called as 'adb bugreport', just falls out to + // the flat-file version. + fprintf(stderr, + "Failed to get bugreportz version, which is only available on devices " + "running Android 7.0 or later.\nTrying a plain-text bug report instead.\n"); + return SendShellCommand(transport_type, serial, "bugreport", false); + } // But if user explicitly asked for a zipped bug report, fails instead (otherwise calling - // 'bugreport' would generate a lot of output the user might not be prepared to handle) + // 'bugreport' would generate a lot of output the user might not be prepared to handle). 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); + "If the device does not run Android 7.0 or above, try 'adb bugreport' instead.\n", + bugz_output.c_str(), status); return status != 0 ? status : -1; } diff --git a/adb/bugreport_test.cpp b/adb/bugreport_test.cpp index f9a0a5e8d..3cd2b6d4f 100644 --- a/adb/bugreport_test.cpp +++ b/adb/bugreport_test.cpp @@ -36,7 +36,9 @@ using ::testing::Return; using ::testing::StrEq; using ::testing::WithArg; using ::testing::internal::CaptureStderr; +using ::testing::internal::CaptureStdout; using ::testing::internal::GetCapturedStderr; +using ::testing::internal::GetCapturedStdout; // Empty function so tests don't need to be linked against file_sync_service.cpp, which requires // SELinux and its transitive dependencies... @@ -152,19 +154,28 @@ class BugreportTest : public ::testing::Test { // Tests when called with invalid number of arguments TEST_F(BugreportTest, InvalidNumberArgs) { - const char* args[1024] = {"bugreport", "to", "principal"}; + const char* args[] = {"bugreport", "to", "principal"}; ASSERT_EQ(-42, br_.DoIt(kTransportLocal, "HannibalLecter", 3, args)); } // Tests the 'adb bugreport' option when the device does not support 'bugreportz' - it falls back // to the flat-file format ('bugreport' binary on device) TEST_F(BugreportTest, NoArgumentsPreNDevice) { - ExpectBugreportzVersion(""); + // clang-format off + EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -v", false, _)) + .WillOnce(DoAll(WithArg<4>(WriteOnStderr("")), + // Write some bogus output on stdout to make sure it's ignored + WithArg<4>(WriteOnStdout("Dude, where is my bugreportz?")), + WithArg<4>(ReturnCallbackDone(0)))); + // clang-format on + std::string bugreport = "Reported the bug was."; + CaptureStdout(); EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreport", false, _)) - .WillOnce(Return(0)); + .WillOnce(DoAll(WithArg<4>(WriteOnStdout(bugreport)), Return(0))); - const char* args[1024] = {"bugreport"}; + const char* args[] = {"bugreport"}; ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 1, args)); + ASSERT_THAT(GetCapturedStdout(), StrEq(bugreport)); } // Tests the 'adb bugreport' option when the device supports 'bugreportz' version 1.0 - it will @@ -181,7 +192,7 @@ TEST_F(BugreportTest, NoArgumentsNDevice) { true, StrEq("generating da_bugreport.zip"))) .WillOnce(Return(true)); - const char* args[1024] = {"bugreport"}; + const char* args[] = {"bugreport"}; ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 1, args)); } @@ -201,7 +212,7 @@ TEST_F(BugreportTest, NoArgumentsPostNDevice) { true, StrEq("generating da_bugreport.zip"))) .WillOnce(Return(true)); - const char* args[1024] = {"bugreport"}; + const char* args[] = {"bugreport"}; ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 1, args)); } @@ -215,7 +226,7 @@ TEST_F(BugreportTest, OkNDevice) { true, StrEq("generating file.zip"))) .WillOnce(Return(true)); - const char* args[1024] = {"bugreport", "file.zip"}; + const char* args[] = {"bugreport", "file.zip"}; ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); } @@ -231,7 +242,7 @@ TEST_F(BugreportTest, OkNDeviceSplitBuffer) { true, StrEq("generating file.zip"))) .WillOnce(Return(true)); - const char* args[1024] = {"bugreport", "file.zip"}; + const char* args[] = {"bugreport", "file.zip"}; ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); } @@ -267,7 +278,7 @@ TEST_F(BugreportTest, OkProgress) { true, StrEq("generating file.zip"))) .WillOnce(Return(true)); - const char* args[1024] = {"bugreport", "file.zip"}; + const char* args[] = {"bugreport", "file.zip"}; ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); } @@ -286,7 +297,7 @@ TEST_F(BugreportTest, OkDirectory) { true, StrEq("generating da_bugreport.zip"))) .WillOnce(Return(true)); - const char* args[1024] = {"bugreport", td.path}; + const char* args[] = {"bugreport", td.path}; ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); } @@ -300,7 +311,7 @@ TEST_F(BugreportTest, OkNoExtension) { true, StrEq("generating file.zip"))) .WillOnce(Return(true)); - const char* args[1024] = {"bugreport", "file"}; + const char* args[] = {"bugreport", "file"}; ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); } @@ -319,7 +330,7 @@ TEST_F(BugreportTest, OkNDeviceDirectory) { true, StrEq("generating da_bugreport.zip"))) .WillOnce(Return(true)); - const char* args[1024] = {"bugreport", td.path}; + const char* args[] = {"bugreport", td.path}; ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); } @@ -331,7 +342,7 @@ TEST_F(BugreportTest, BugreportzReturnedFail) { DoAll(WithArg<4>(WriteOnStdout("FAIL:D'OH!\n")), WithArg<4>(ReturnCallbackDone()))); CaptureStderr(); - const char* args[1024] = {"bugreport", "file.zip"}; + const char* args[] = {"bugreport", "file.zip"}; ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); ASSERT_THAT(GetCapturedStderr(), HasSubstr("D'OH!")); } @@ -346,7 +357,7 @@ TEST_F(BugreportTest, BugreportzReturnedFailSplitBuffer) { WithArg<4>(ReturnCallbackDone()))); CaptureStderr(); - const char* args[1024] = {"bugreport", "file.zip"}; + const char* args[] = {"bugreport", "file.zip"}; ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); ASSERT_THAT(GetCapturedStderr(), HasSubstr("D'OH!")); } @@ -360,7 +371,7 @@ TEST_F(BugreportTest, BugreportzReturnedUnsupported) { WithArg<4>(ReturnCallbackDone()))); CaptureStderr(); - const char* args[1024] = {"bugreport", "file.zip"}; + const char* args[] = {"bugreport", "file.zip"}; ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); ASSERT_THAT(GetCapturedStderr(), HasSubstr("bugreportz? What am I, a zombie?")); } @@ -370,7 +381,7 @@ TEST_F(BugreportTest, BugreportzVersionFailed) { EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -v", false, _)) .WillOnce(Return(666)); - const char* args[1024] = {"bugreport", "file.zip"}; + const char* args[] = {"bugreport", "file.zip"}; ASSERT_EQ(666, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); } @@ -378,7 +389,7 @@ TEST_F(BugreportTest, BugreportzVersionFailed) { TEST_F(BugreportTest, BugreportzVersionEmpty) { ExpectBugreportzVersion(""); - const char* args[1024] = {"bugreport", "file.zip"}; + const char* args[] = {"bugreport", "file.zip"}; ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); } @@ -388,7 +399,7 @@ TEST_F(BugreportTest, BugreportzFailed) { EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _)) .WillOnce(Return(666)); - const char* args[1024] = {"bugreport", "file.zip"}; + const char* args[] = {"bugreport", "file.zip"}; ASSERT_EQ(666, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); } @@ -402,6 +413,6 @@ TEST_F(BugreportTest, PullFails) { true, HasSubstr("file.zip"))) .WillOnce(Return(false)); - const char* args[1024] = {"bugreport", "file.zip"}; + const char* args[] = {"bugreport", "file.zip"}; ASSERT_EQ(1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); }