Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 904e0e0f authored by Abhijeet Kaur's avatar Abhijeet Kaur
Browse files

Fix adb bugreport and add bugreport unit tests. 

Commit 58d72e22 introduced a regression, which made fullbugreport as the
default mode. This lead to 'adb bugreport SAMPLE_BR' triggering a
progress notification on the device as it inherited do_broadcast from full bugreport.

Fix the issue by separating full bugreport from default bugreport.

Add unit tests for all the cases of calling bugreport.

Bug: 119877616
Test: Verified that 'adb bugreport SAMPLE_BR' does not show any notification on the device.
Test: mmm -j frameworks/native/cmds/dumpstate/ && adb push ${OUT}/data/nativetest64/dumpstate_test* /data/nativetest64 && adb shell /data/nativetest64/dumpstate_test/dumpstate_test && printf "\n\n#### ALL TESTS PASSED ####\n"

Change-Id: I06e57454a91a3276dd89d47b3b0a04d3d85f32da
parent 5f7692f5

cmds/dumpstate/DumpstateService.cpp

+2 −1
Original line number Diff line number Diff line
@@ -108,7 +108,8 @@ binder::Status DumpstateService::startBugreport(int, int bugreport_mode, int32_t 
        bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_REMOTE &&

        bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_WEAR &&

        bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_TELEPHONY &&

        bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_WIFI) {

        bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_WIFI &&

        bugreport_mode != Dumpstate::BugreportMode::BUGREPORT_DEFAULT) {

        return exception(binder::Status::EX_ILLEGAL_ARGUMENT,

                         StringPrintf("Invalid bugreport mode: %d", bugreport_mode));

    }

cmds/dumpstate/binder/android/os/IDumpstate.aidl

+4 −1
Original line number Diff line number Diff line
@@ -40,7 +40,7 @@ interface IDumpstate { 
                                boolean getSectionDetails);



    // These modes encapsulate a set of run time options for generating bugreports.

    // A zipped bugreport; default mode.

    // Takes a bugreport without user interference.

    const int BUGREPORT_MODE_FULL = 0;



    // Interactive bugreport, i.e. triggered by the user.
@@ -58,6 +58,9 @@ interface IDumpstate { 
    // Bugreport limited to only wifi info.

    const int BUGREPORT_MODE_WIFI = 5;



    // Default mode.

    const int BUGREPORT_MODE_DEFAULT = 6;



    /*

     * Starts a bugreport in the background.

     */

cmds/dumpstate/dumpstate.cpp

+8 −2
Original line number Diff line number Diff line
@@ -1948,6 +1948,8 @@ static inline const char* ModeToString(Dumpstate::BugreportMode mode) { 
            return "BUGREPORT_TELEPHONY";

        case Dumpstate::BugreportMode::BUGREPORT_WIFI:

            return "BUGREPORT_WIFI";

        case Dumpstate::BugreportMode::BUGREPORT_DEFAULT:

            return "BUGREPORT_DEFAULT";

    }

}
@@ -1988,12 +1990,14 @@ static void SetOptionsFromMode(Dumpstate::BugreportMode mode, Dumpstate::DumpOpt 
            options->do_fb = true;

            options->do_broadcast = true;

            break;

        case Dumpstate::BugreportMode::BUGREPORT_DEFAULT:

            break;

    }

}



static Dumpstate::BugreportMode getBugreportModeFromProperty() {

    // If the system property is not set, it's assumed to be a full bugreport.

    Dumpstate::BugreportMode mode = Dumpstate::BugreportMode::BUGREPORT_FULL;

    // If the system property is not set, it's assumed to be a default bugreport.

    Dumpstate::BugreportMode mode = Dumpstate::BugreportMode::BUGREPORT_DEFAULT;



    std::string extra_options = android::base::GetProperty(PROPERTY_EXTRA_OPTIONS, "");

    if (!extra_options.empty()) {
@@ -2001,6 +2005,8 @@ static Dumpstate::BugreportMode getBugreportModeFromProperty() { 
        // Currently, it contains the type of the requested bugreport.

        if (extra_options == "bugreportplus") {

            mode = Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE;

        } else if (extra_options == "bugreportfull") {

            mode = Dumpstate::BugreportMode::BUGREPORT_FULL;

        } else if (extra_options == "bugreportremote") {

            mode = Dumpstate::BugreportMode::BUGREPORT_REMOTE;

        } else if (extra_options == "bugreportwear") {

cmds/dumpstate/dumpstate.h

+2 −1
Original line number Diff line number Diff line
@@ -196,7 +196,8 @@ class Dumpstate { 
        BUGREPORT_REMOTE = android::os::IDumpstate::BUGREPORT_MODE_REMOTE,

        BUGREPORT_WEAR = android::os::IDumpstate::BUGREPORT_MODE_WEAR,

        BUGREPORT_TELEPHONY = android::os::IDumpstate::BUGREPORT_MODE_TELEPHONY,

        BUGREPORT_WIFI = android::os::IDumpstate::BUGREPORT_MODE_WIFI

        BUGREPORT_WIFI = android::os::IDumpstate::BUGREPORT_MODE_WIFI,

        BUGREPORT_DEFAULT = android::os::IDumpstate::BUGREPORT_MODE_DEFAULT

    };



    static android::os::dumpstate::CommandOptions DEFAULT_DUMPSYS;

cmds/dumpstate/tests/dumpstate_test.cpp

+292 −4
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ 
#include <android-base/properties.h>

#include <android-base/stringprintf.h>

#include <android-base/strings.h>

#include <cutils/properties.h>



namespace android {

namespace os {
@@ -148,7 +149,10 @@ class DumpOptionsTest : public Test { 
    virtual void SetUp() {

        options_ = Dumpstate::DumpOptions();

    }



    void TearDown() {

        // Reset the property

        property_set("dumpstate.options", "");

    }

    Dumpstate::DumpOptions options_;

};
@@ -163,7 +167,6 @@ TEST_F(DumpOptionsTest, InitializeNone) { 


    EXPECT_EQ(status, Dumpstate::RunStatus::OK);



    // These correspond to bugreport_mode = full, because that's the default.

    EXPECT_FALSE(options_.do_add_date);

    EXPECT_FALSE(options_.do_zip_file);

    EXPECT_EQ("", options_.use_outfile);
@@ -171,10 +174,295 @@ TEST_F(DumpOptionsTest, InitializeNone) { 
    EXPECT_FALSE(options_.use_control_socket);

    EXPECT_FALSE(options_.show_header_only);

    EXPECT_TRUE(options_.do_vibrate);

    EXPECT_FALSE(options_.do_fb);

    EXPECT_FALSE(options_.do_progress_updates);

    EXPECT_FALSE(options_.is_remote_mode);

    EXPECT_FALSE(options_.do_broadcast);

}



TEST_F(DumpOptionsTest, InitializeAdbBugreport) {

    // clang-format off

    char* argv[] = {

        const_cast<char*>("dumpstatez"),

        const_cast<char*>("-S"),

        const_cast<char*>("-d"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on



    Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);



    EXPECT_EQ(status, Dumpstate::RunStatus::OK);

    EXPECT_TRUE(options_.do_add_date);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.use_control_socket);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);

    EXPECT_FALSE(options_.show_header_only);

    EXPECT_FALSE(options_.do_fb);

    EXPECT_FALSE(options_.do_progress_updates);

    EXPECT_FALSE(options_.is_remote_mode);

    EXPECT_FALSE(options_.do_broadcast);

    EXPECT_FALSE(options_.use_socket);

}



TEST_F(DumpOptionsTest, InitializeAdbShellBugreport) {

    // clang-format off

    char* argv[] = {

        const_cast<char*>("dumpstate"),

        const_cast<char*>("-s"),

    };

    // clang-format on



    Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);



    EXPECT_EQ(status, Dumpstate::RunStatus::OK);

    EXPECT_TRUE(options_.use_socket);



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);

    EXPECT_EQ("", options_.use_outfile);

    EXPECT_FALSE(options_.do_add_date);

    EXPECT_FALSE(options_.do_zip_file);

    EXPECT_FALSE(options_.use_control_socket);

    EXPECT_FALSE(options_.show_header_only);

    EXPECT_FALSE(options_.do_fb);

    EXPECT_FALSE(options_.do_progress_updates);

    EXPECT_FALSE(options_.is_remote_mode);

    EXPECT_FALSE(options_.do_broadcast);

}



TEST_F(DumpOptionsTest, InitializeFullBugReport) {

    // clang-format off

    char* argv[] = {

        const_cast<char*>("bugreport"),

        const_cast<char*>("-d"),

        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on

    property_set("dumpstate.options", "bugreportfull");



    Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);



    EXPECT_EQ(status, Dumpstate::RunStatus::OK);

    EXPECT_TRUE(options_.do_add_date);

    EXPECT_TRUE(options_.do_fb);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.do_broadcast);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);

    EXPECT_FALSE(options_.use_control_socket);

    EXPECT_FALSE(options_.show_header_only);

    EXPECT_FALSE(options_.do_progress_updates);

    EXPECT_FALSE(options_.is_remote_mode);

    EXPECT_FALSE(options_.use_socket);

    EXPECT_FALSE(options_.do_start_service);

}



TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) {

    // clang-format off

    char* argv[] = {

        const_cast<char*>("bugreport"),

        const_cast<char*>("-d"),

        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on



    property_set("dumpstate.options", "bugreportplus");



    Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);



    EXPECT_EQ(status, Dumpstate::RunStatus::OK);

    EXPECT_TRUE(options_.do_add_date);

    EXPECT_TRUE(options_.do_broadcast);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.do_progress_updates);

    EXPECT_TRUE(options_.do_start_service);

    EXPECT_FALSE(options_.do_fb);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);

    EXPECT_FALSE(options_.use_control_socket);

    EXPECT_FALSE(options_.show_header_only);

    EXPECT_FALSE(options_.is_remote_mode);

    EXPECT_FALSE(options_.use_socket);

}



TEST_F(DumpOptionsTest, InitializeRemoteBugReport) {

    // clang-format off

    char* argv[] = {

        const_cast<char*>("bugreport"),

        const_cast<char*>("-d"),

        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on



    property_set("dumpstate.options", "bugreportremote");



    Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);



    EXPECT_EQ(status, Dumpstate::RunStatus::OK);

    EXPECT_TRUE(options_.do_add_date);

    EXPECT_TRUE(options_.do_broadcast);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.is_remote_mode);

    EXPECT_FALSE(options_.do_vibrate);

    EXPECT_FALSE(options_.do_fb);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_FALSE(options_.use_control_socket);

    EXPECT_FALSE(options_.show_header_only);

    EXPECT_FALSE(options_.do_progress_updates);

    EXPECT_FALSE(options_.use_socket);

}



TEST_F(DumpOptionsTest, InitializeWearBugReport) {

    // clang-format off

    char* argv[] = {

        const_cast<char*>("bugreport"),

        const_cast<char*>("-d"),

        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on



    property_set("dumpstate.options", "bugreportwear");



    Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);



    EXPECT_EQ(status, Dumpstate::RunStatus::OK);

    EXPECT_TRUE(options_.do_add_date);

    EXPECT_TRUE(options_.do_fb);

    EXPECT_TRUE(options_.do_broadcast);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.do_progress_updates);

    EXPECT_TRUE(options_.do_start_service);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);

    EXPECT_FALSE(options_.use_control_socket);

    EXPECT_FALSE(options_.show_header_only);

    EXPECT_FALSE(options_.is_remote_mode);

    EXPECT_FALSE(options_.use_socket);

}



TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) {

    // clang-format off

    char* argv[] = {

        const_cast<char*>("bugreport"),

        const_cast<char*>("-d"),

        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on



    property_set("dumpstate.options", "bugreporttelephony");



    Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);



    EXPECT_EQ(status, Dumpstate::RunStatus::OK);

    EXPECT_TRUE(options_.do_add_date);

    EXPECT_TRUE(options_.do_fb);

    EXPECT_TRUE(options_.do_broadcast);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.telephony_only);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);

    EXPECT_FALSE(options_.use_control_socket);

    EXPECT_FALSE(options_.show_header_only);

    EXPECT_FALSE(options_.do_progress_updates);

    EXPECT_FALSE(options_.is_remote_mode);

    EXPECT_FALSE(options_.use_socket);

}



TEST_F(DumpOptionsTest, InitializeWifiBugReport) {

    // clang-format off

    char* argv[] = {

        const_cast<char*>("bugreport"),

        const_cast<char*>("-d"),

        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on



    property_set("dumpstate.options", "bugreportwifi");



    Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);



    EXPECT_EQ(status, Dumpstate::RunStatus::OK);

    EXPECT_TRUE(options_.do_add_date);

    EXPECT_TRUE(options_.do_fb);

    EXPECT_TRUE(options_.do_broadcast);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.wifi_only);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);

    EXPECT_FALSE(options_.use_control_socket);

    EXPECT_FALSE(options_.show_header_only);

    EXPECT_FALSE(options_.do_progress_updates);

    EXPECT_FALSE(options_.is_remote_mode);

    EXPECT_FALSE(options_.use_socket);

}



TEST_F(DumpOptionsTest, InitializeDefaultBugReport) {

    // default: commandline options are not overridden

    // clang-format off

    char* argv[] = {

        const_cast<char*>("bugreport"),

        const_cast<char*>("-d"),

        const_cast<char*>("-p"),

        const_cast<char*>("-B"),

        const_cast<char*>("-z"),

        const_cast<char*>("-o abc"),

    };

    // clang-format on



    property_set("dumpstate.options", "");



    Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);



    EXPECT_EQ(status, Dumpstate::RunStatus::OK);

    EXPECT_TRUE(options_.do_add_date);

    EXPECT_TRUE(options_.do_fb);

    EXPECT_TRUE(options_.do_zip_file);

    EXPECT_TRUE(options_.do_broadcast);

    EXPECT_EQ(" abc", std::string(options_.use_outfile));



    // Other options retain default values

    EXPECT_TRUE(options_.do_vibrate);

    EXPECT_FALSE(options_.use_control_socket);

    EXPECT_FALSE(options_.show_header_only);

    EXPECT_FALSE(options_.do_progress_updates);

    EXPECT_FALSE(options_.is_remote_mode);

    EXPECT_FALSE(options_.use_socket);

    EXPECT_FALSE(options_.wifi_only);

}



TEST_F(DumpOptionsTest, InitializePartial1) {
@@ -203,10 +491,10 @@ TEST_F(DumpOptionsTest, InitializePartial1) { 
    // Other options retain default values

    EXPECT_FALSE(options_.show_header_only);

    EXPECT_TRUE(options_.do_vibrate);

    EXPECT_TRUE(options_.do_fb);

    EXPECT_FALSE(options_.do_fb);

    EXPECT_FALSE(options_.do_progress_updates);

    EXPECT_FALSE(options_.is_remote_mode);

    EXPECT_TRUE(options_.do_broadcast);

    EXPECT_FALSE(options_.do_broadcast);

}



TEST_F(DumpOptionsTest, InitializePartial2) {