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

Commit fdfffe15 authored by Treehugger Robot's avatar Treehugger Robot Committed by Gerrit Code Review
Browse files

Merge "Separate options parsing from running dumpstate"

parents 31208f1b 5fb117bb
Loading
Loading
Loading
Loading
+6 −6
Original line number Diff line number Diff line
@@ -87,27 +87,27 @@ binder::Status DumpstateService::startBugreport(int, const sp<IDumpstateListener
status_t DumpstateService::dump(int fd, const Vector<String16>&) {
    dprintf(fd, "id: %d\n", ds_.id_);
    dprintf(fd, "pid: %d\n", ds_.pid_);
    dprintf(fd, "update_progress: %s\n", ds_.update_progress_ ? "true" : "false");
    dprintf(fd, "update_progress: %s\n", ds_.options_->do_progress_updates ? "true" : "false");
    dprintf(fd, "update_progress_threshold: %d\n", ds_.update_progress_threshold_);
    dprintf(fd, "last_updated_progress: %d\n", ds_.last_updated_progress_);
    dprintf(fd, "progress:\n");
    ds_.progress_->Dump(fd, "  ");
    dprintf(fd, "args: %s\n", ds_.args_.c_str());
    dprintf(fd, "extra_options: %s\n", ds_.extra_options_.c_str());
    dprintf(fd, "args: %s\n", ds_.options_->args.c_str());
    dprintf(fd, "extra_options: %s\n", ds_.options_->extra_options.c_str());
    dprintf(fd, "version: %s\n", ds_.version_.c_str());
    dprintf(fd, "bugreport_dir: %s\n", ds_.bugreport_dir_.c_str());
    dprintf(fd, "screenshot_path: %s\n", ds_.screenshot_path_.c_str());
    dprintf(fd, "log_path: %s\n", ds_.log_path_.c_str());
    dprintf(fd, "tmp_path: %s\n", ds_.tmp_path_.c_str());
    dprintf(fd, "path: %s\n", ds_.path_.c_str());
    dprintf(fd, "extra_options: %s\n", ds_.extra_options_.c_str());
    dprintf(fd, "extra_options: %s\n", ds_.options_->extra_options.c_str());
    dprintf(fd, "base_name: %s\n", ds_.base_name_.c_str());
    dprintf(fd, "name: %s\n", ds_.name_.c_str());
    dprintf(fd, "now: %ld\n", ds_.now_);
    dprintf(fd, "is_zipping: %s\n", ds_.IsZipping() ? "true" : "false");
    dprintf(fd, "listener: %s\n", ds_.listener_name_.c_str());
    dprintf(fd, "notification title: %s\n", ds_.notification_title.c_str());
    dprintf(fd, "notification description: %s\n", ds_.notification_description.c_str());
    dprintf(fd, "notification title: %s\n", ds_.options_->notification_title.c_str());
    dprintf(fd, "notification description: %s\n", ds_.options_->notification_description.c_str());

    return NO_ERROR;
}
+185 −168

File changed.

Preview size limit exceeded, changes collapsed.

+22 −28
Original line number Diff line number Diff line
@@ -183,6 +183,8 @@ class Dumpstate {
    friend class DumpstateTest;

  public:
    enum RunStatus { OK, HELP, INVALID_INPUT, ERROR };

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

    static Dumpstate& GetInstance();
@@ -290,22 +292,12 @@ class Dumpstate {
    /* Returns true if the current version supports priority dump feature. */
    bool CurrentVersionSupportsPriorityDumps() const;

    // TODO: revisit the return values later.
    /*
     * Parses commandline arguments and sets runtime options accordingly.
     *
     * Returns 0 or positive number if the caller should exit with returned value as
     * exit code, or returns -1 if caller should proceed with execution.
     */
    int ParseCommandlineOptions(int argc, char* argv[]);
    struct DumpOptions;

    /* Sets runtime options from the system properties. */
    void SetOptionsFromProperties();
    /* Main entry point for running a complete bugreport. Takes ownership of options. */
    RunStatus RunWithOptions(std::unique_ptr<DumpOptions> options);

    /* Returns true if the options set so far are consistent. */
    bool ValidateOptions();

    // TODO: add update_progress_ & other options from DumpState.
    // TODO: add other options from DumpState.
    /*
     * Structure to hold options that determine the behavior of dumpstate.
     */
@@ -322,7 +314,22 @@ class Dumpstate {
        bool do_start_service = false;
        bool telephony_only = false;
        bool wifi_only = false;
        // Whether progress updates should be published.
        bool do_progress_updates = false;
        std::string use_outfile;
        // Extra options passed as system property.
        std::string extra_options;
        // Command-line arguments as string
        std::string args;
        // Notification title and description
        std::string notification_title;
        std::string notification_description;

        /* Initializes options from commandline arguments and system properties. */
        RunStatus Initialize(int argc, char* argv[]);

        /* Returns true if the options set so far are consistent. */
        bool ValidateOptions() const;
    };

    // TODO: initialize fields on constructor
@@ -333,10 +340,7 @@ class Dumpstate {
    pid_t pid_;

    // Runtime options.
    DumpOptions options_;

    // Whether progress updates should be published.
    bool update_progress_ = false;
    std::unique_ptr<DumpOptions> options_;

    // How frequently the progess should be updated;the listener will only be notificated when the
    // delta from the previous update is more than the threshold.
@@ -356,12 +360,6 @@ class Dumpstate {
    // Bugreport format version;
    std::string version_ = VERSION_CURRENT;

    // Command-line arguments as string
    std::string args_;

    // Extra options passed as system property.
    std::string extra_options_;

    // Full path of the directory where the bugreport files will be written.
    std::string bugreport_dir_;

@@ -398,10 +396,6 @@ class Dumpstate {
    std::string listener_name_;
    bool report_section_;

    // Notification title and description
    std::string notification_title;
    std::string notification_description;

    // List of open tombstone dump files.
    std::vector<DumpData> tombstone_data_;

+161 −144
Original line number Diff line number Diff line
@@ -137,96 +137,42 @@ class DumpstateBaseTest : public Test {
    }
};

class DumpstateTest : public DumpstateBaseTest {
class DumpOptionsTest : public Test {
  public:
    void SetUp() {
        DumpstateBaseTest::SetUp();
        SetDryRun(false);
        SetBuildType(android::base::GetProperty("ro.build.type", "(unknown)"));
        ds.progress_.reset(new Progress());
        ds.update_progress_ = false;
        ds.update_progress_threshold_ = 0;
        ds.options_ = Dumpstate::DumpOptions();
    }

    // Runs a command and capture `stdout` and `stderr`.
    int RunCommand(const std::string& title, const std::vector<std::string>& full_command,
                   const CommandOptions& options = CommandOptions::DEFAULT) {
        CaptureStdout();
        CaptureStderr();
        int status = ds.RunCommand(title, full_command, options);
        out = GetCapturedStdout();
        err = GetCapturedStderr();
        return status;
    }

    // Dumps a file and capture `stdout` and `stderr`.
    int DumpFile(const std::string& title, const std::string& path) {
        CaptureStdout();
        CaptureStderr();
        int status = ds.DumpFile(title, path);
        out = GetCapturedStdout();
        err = GetCapturedStderr();
        return status;
    }

    void SetProgress(long progress, long initial_max, long threshold = 0) {
        ds.update_progress_ = true;
        ds.update_progress_threshold_ = threshold;
        ds.last_updated_progress_ = 0;
        ds.progress_.reset(new Progress(initial_max, progress, 1.2));
    virtual ~DumpOptionsTest() {
    }

    std::string GetProgressMessage(const std::string& listener_name, int progress, int max,
                                   int old_max = 0, bool update_progress = true) {
        EXPECT_EQ(progress, ds.progress_->Get()) << "invalid progress";
        EXPECT_EQ(max, ds.progress_->GetMax()) << "invalid max";

        bool max_increased = old_max > 0;

        std::string message = "";
        if (max_increased) {
            message =
                android::base::StringPrintf("Adjusting max progress from %d to %d\n", old_max, max);
    virtual void SetUp() {
        options_ = Dumpstate::DumpOptions();
    }

        if (update_progress) {
            message += android::base::StringPrintf("Setting progress (%s): %d/%d\n",
                                                   listener_name.c_str(), progress, max);
        }

        return message;
    }

    // `stdout` and `stderr` from the last command ran.
    std::string out, err;

    Dumpstate& ds = Dumpstate::GetInstance();
    Dumpstate::DumpOptions options_;
};

TEST_F(DumpstateTest, ParseCommandlineOptionsNone) {
TEST_F(DumpOptionsTest, InitializeNone) {
    // clang-format off
    char* argv[] = {
        const_cast<char*>("dumpstate")
    };
    // clang-format on

    int ret = ds.ParseCommandlineOptions(ARRAY_SIZE(argv), argv);
    EXPECT_EQ(-1, ret);
    EXPECT_FALSE(ds.options_.do_add_date);
    EXPECT_FALSE(ds.options_.do_zip_file);
    EXPECT_EQ("", ds.options_.use_outfile);
    EXPECT_FALSE(ds.options_.use_socket);
    EXPECT_FALSE(ds.options_.use_control_socket);
    EXPECT_FALSE(ds.options_.show_header_only);
    EXPECT_TRUE(ds.options_.do_vibrate);
    EXPECT_FALSE(ds.options_.do_fb);
    EXPECT_FALSE(ds.update_progress_);
    EXPECT_FALSE(ds.options_.is_remote_mode);
    EXPECT_FALSE(ds.options_.do_broadcast);
}

TEST_F(DumpstateTest, ParseCommandlineOptionsPartial1) {
    Dumpstate::DumpOptions options;
    Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);

    EXPECT_EQ(status, Dumpstate::RunStatus::OK);
    EXPECT_FALSE(options_.do_add_date);
    EXPECT_FALSE(options_.do_zip_file);
    EXPECT_EQ("", options_.use_outfile);
    EXPECT_FALSE(options_.use_socket);
    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, InitializePartial1) {
    // clang-format off
    char* argv[] = {
        const_cast<char*>("dumpstate"),
@@ -238,25 +184,27 @@ TEST_F(DumpstateTest, ParseCommandlineOptionsPartial1) {

    };
    // clang-format on
    int ret = ds.ParseCommandlineOptions(ARRAY_SIZE(argv), argv);
    EXPECT_EQ(-1, ret);
    EXPECT_TRUE(ds.options_.do_add_date);
    EXPECT_TRUE(ds.options_.do_zip_file);

    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);
    // TODO: Maybe we should trim the filename
    EXPECT_EQ(" abc", std::string(ds.options_.use_outfile));
    EXPECT_TRUE(ds.options_.use_socket);
    EXPECT_TRUE(ds.options_.use_control_socket);
    EXPECT_EQ(" abc", std::string(options_.use_outfile));
    EXPECT_TRUE(options_.use_socket);
    EXPECT_TRUE(options_.use_control_socket);

    // Other options retain default values
    EXPECT_FALSE(ds.options_.show_header_only);
    EXPECT_TRUE(ds.options_.do_vibrate);
    EXPECT_FALSE(ds.options_.do_fb);
    EXPECT_FALSE(ds.update_progress_);
    EXPECT_FALSE(ds.options_.is_remote_mode);
    EXPECT_FALSE(ds.options_.do_broadcast);
    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(DumpstateTest, ParseCommandlineOptionsPartial2) {
TEST_F(DumpOptionsTest, InitializePartial2) {
    // clang-format off
    char* argv[] = {
        const_cast<char*>("dumpstate"),
@@ -268,93 +216,162 @@ TEST_F(DumpstateTest, ParseCommandlineOptionsPartial2) {
        const_cast<char*>("-B"),
    };
    // clang-format on
    int ret = ds.ParseCommandlineOptions(ARRAY_SIZE(argv), argv);
    EXPECT_EQ(-1, ret);
    EXPECT_TRUE(ds.options_.show_header_only);
    EXPECT_FALSE(ds.options_.do_vibrate);
    EXPECT_TRUE(ds.options_.do_fb);
    EXPECT_TRUE(ds.update_progress_);
    EXPECT_TRUE(ds.options_.is_remote_mode);
    EXPECT_TRUE(ds.options_.do_broadcast);

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

    EXPECT_EQ(status, Dumpstate::RunStatus::OK);
    EXPECT_TRUE(options_.show_header_only);
    EXPECT_FALSE(options_.do_vibrate);
    EXPECT_TRUE(options_.do_fb);
    EXPECT_TRUE(options_.do_progress_updates);
    EXPECT_TRUE(options_.is_remote_mode);
    EXPECT_TRUE(options_.do_broadcast);

    // Other options retain default values
    EXPECT_FALSE(ds.options_.do_add_date);
    EXPECT_FALSE(ds.options_.do_zip_file);
    EXPECT_EQ("", ds.options_.use_outfile);
    EXPECT_FALSE(ds.options_.use_socket);
    EXPECT_FALSE(ds.options_.use_control_socket);
    EXPECT_FALSE(options_.do_add_date);
    EXPECT_FALSE(options_.do_zip_file);
    EXPECT_EQ("", options_.use_outfile);
    EXPECT_FALSE(options_.use_socket);
    EXPECT_FALSE(options_.use_control_socket);
}

TEST_F(DumpstateTest, ParseCommandlineOptionsHelp) {
TEST_F(DumpOptionsTest, InitializeHelp) {
    // clang-format off
    char* argv[] = {
        const_cast<char*>("dumpstate"),
        const_cast<char*>("-h")
    };
    // clang-format on
    int ret = ds.ParseCommandlineOptions(ARRAY_SIZE(argv), argv);

    // -h is for help. Caller exit with code = 0 after printing usage, so expect return = 0.
    EXPECT_EQ(0, ret);
    Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);

    // -h is for help.
    EXPECT_EQ(status, Dumpstate::RunStatus::HELP);
}

TEST_F(DumpstateTest, ParseCommandlineOptionsUnknown) {
TEST_F(DumpOptionsTest, InitializeUnknown) {
    // clang-format off
    char* argv[] = {
        const_cast<char*>("dumpstate"),
        const_cast<char*>("-u")  // unknown flag
    };
    // clang-format on
    int ret = ds.ParseCommandlineOptions(ARRAY_SIZE(argv), argv);

    // -u is unknown. Caller exit with code = 1 to show execution failure, after printing usage,
    // so expect return = 1.
    EXPECT_EQ(1, ret);
    Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);

    // -u is unknown.
    EXPECT_EQ(status, Dumpstate::RunStatus::INVALID_INPUT);
}

TEST_F(DumpstateTest, ValidateOptionsNeedOutfile1) {
    ds.options_.do_zip_file = true;
    EXPECT_FALSE(ds.ValidateOptions());
    ds.options_.use_outfile = "a/b/c";
    EXPECT_TRUE(ds.ValidateOptions());
TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile1) {
    options_.do_zip_file = true;
    EXPECT_FALSE(options_.ValidateOptions());
    options_.use_outfile = "a/b/c";
    EXPECT_TRUE(options_.ValidateOptions());
}

TEST_F(DumpstateTest, ValidateOptionsNeedOutfile2) {
    ds.options_.do_broadcast = true;
    EXPECT_FALSE(ds.ValidateOptions());
    ds.options_.use_outfile = "a/b/c";
    EXPECT_TRUE(ds.ValidateOptions());
TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile2) {
    options_.do_broadcast = true;
    EXPECT_FALSE(options_.ValidateOptions());
    options_.use_outfile = "a/b/c";
    EXPECT_TRUE(options_.ValidateOptions());
}

TEST_F(DumpstateTest, ValidateOptionsNeedZipfile) {
    ds.options_.use_control_socket = true;
    EXPECT_FALSE(ds.ValidateOptions());
TEST_F(DumpOptionsTest, ValidateOptionsNeedZipfile) {
    options_.use_control_socket = true;
    EXPECT_FALSE(options_.ValidateOptions());

    ds.options_.do_zip_file = true;
    ds.options_.use_outfile = "a/b/c";  // do_zip_file needs outfile
    EXPECT_TRUE(ds.ValidateOptions());
    options_.do_zip_file = true;
    options_.use_outfile = "a/b/c";  // do_zip_file needs outfile
    EXPECT_TRUE(options_.ValidateOptions());
}

TEST_F(DumpstateTest, ValidateOptionsUpdateProgressNeedsBroadcast) {
    ds.update_progress_ = true;
    ds.options_.use_outfile = "a/b/c";  // update_progress_ needs outfile
    EXPECT_FALSE(ds.ValidateOptions());
TEST_F(DumpOptionsTest, ValidateOptionsUpdateProgressNeedsBroadcast) {
    options_.do_progress_updates = true;
    options_.use_outfile = "a/b/c";  // do_progress_updates needs outfile
    EXPECT_FALSE(options_.ValidateOptions());

    ds.options_.do_broadcast = true;
    EXPECT_TRUE(ds.ValidateOptions());
    options_.do_broadcast = true;
    EXPECT_TRUE(options_.ValidateOptions());
}

TEST_F(DumpstateTest, ValidateOptionsRemoteMode) {
    ds.options_.is_remote_mode = true;
    EXPECT_FALSE(ds.ValidateOptions());
TEST_F(DumpOptionsTest, ValidateOptionsRemoteMode) {
    options_.is_remote_mode = true;
    EXPECT_FALSE(options_.ValidateOptions());

    ds.options_.do_broadcast = true;
    ds.options_.do_zip_file = true;
    ds.options_.do_add_date = true;
    ds.options_.use_outfile = "a/b/c";  // do_broadcast needs outfile
    EXPECT_TRUE(ds.ValidateOptions());
    options_.do_broadcast = true;
    options_.do_zip_file = true;
    options_.do_add_date = true;
    options_.use_outfile = "a/b/c";  // do_broadcast needs outfile
    EXPECT_TRUE(options_.ValidateOptions());
}

class DumpstateTest : public DumpstateBaseTest {
  public:
    void SetUp() {
        DumpstateBaseTest::SetUp();
        SetDryRun(false);
        SetBuildType(android::base::GetProperty("ro.build.type", "(unknown)"));
        ds.progress_.reset(new Progress());
        ds.update_progress_threshold_ = 0;
        ds.options_.reset(new Dumpstate::DumpOptions());
    }

    // Runs a command and capture `stdout` and `stderr`.
    int RunCommand(const std::string& title, const std::vector<std::string>& full_command,
                   const CommandOptions& options = CommandOptions::DEFAULT) {
        CaptureStdout();
        CaptureStderr();
        int status = ds.RunCommand(title, full_command, options);
        out = GetCapturedStdout();
        err = GetCapturedStderr();
        return status;
    }

    // Dumps a file and capture `stdout` and `stderr`.
    int DumpFile(const std::string& title, const std::string& path) {
        CaptureStdout();
        CaptureStderr();
        int status = ds.DumpFile(title, path);
        out = GetCapturedStdout();
        err = GetCapturedStderr();
        return status;
    }

    void SetProgress(long progress, long initial_max, long threshold = 0) {
        ds.options_->do_progress_updates = true;
        ds.update_progress_threshold_ = threshold;
        ds.last_updated_progress_ = 0;
        ds.progress_.reset(new Progress(initial_max, progress, 1.2));
    }

    std::string GetProgressMessage(const std::string& listener_name, int progress, int max,
                                   int old_max = 0, bool update_progress = true) {
        EXPECT_EQ(progress, ds.progress_->Get()) << "invalid progress";
        EXPECT_EQ(max, ds.progress_->GetMax()) << "invalid max";

        bool max_increased = old_max > 0;

        std::string message = "";
        if (max_increased) {
            message =
                android::base::StringPrintf("Adjusting max progress from %d to %d\n", old_max, max);
        }

        if (update_progress) {
            message += android::base::StringPrintf("Setting progress (%s): %d/%d\n",
                                                   listener_name.c_str(), progress, max);
        }

        return message;
    }

    // `stdout` and `stderr` from the last command ran.
    std::string out, err;

    Dumpstate& ds = Dumpstate::GetInstance();
};

TEST_F(DumpstateTest, RunCommandNoArgs) {
    EXPECT_EQ(-1, RunCommand("", {}));
}
+1 −1
Original line number Diff line number Diff line
@@ -916,7 +916,7 @@ void Dumpstate::UpdateProgress(int32_t delta_sec) {
    bool max_changed = progress_->Inc(delta_sec);

    // ...but only notifiy listeners when necessary.
    if (!update_progress_) return;
    if (!options_->do_progress_updates) return;

    int progress = progress_->Get();
    int max = progress_->GetMax();