Moar Dumpstate refactoring and unit testing:
- Moves UpdateProgress() into Dumpstate object.
- Tests RunCommand.AsRoot().
- Tests RunCommand.DropRoot().
- Test update progress.
BUG: 26379932
BUG: 31807540
Test: mmm -j32 frameworks/native/cmds/dumpstate/ && adb push ${ANDROID_PRODUCT_OUT}/data/nativetest/dumpstate_test* /data/nativetest && adb shell /data/nativetest/dumpstate_test/dumpstate_test
Change-Id: I364d097487e090d201eb2e5f08fc794dce555510
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 264251d..32dbf8d 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -52,8 +52,6 @@
#include <openssl/sha.h>
-using android::base::StringPrintf;
-
/* read before root is shed */
static char cmdline_buf[16384] = "(unknown)";
static const char *dump_traces_path = NULL;
@@ -695,7 +693,6 @@
build = android::base::GetProperty("ro.build.display.id", "(unknown)");
fingerprint = android::base::GetProperty("ro.build.fingerprint", "(unknown)");
- ds.buildType_ = android::base::GetProperty("ro.build.type", "(unknown)");
radio = android::base::GetProperty("gsm.version.baseband", "(unknown)");
bootloader = android::base::GetProperty("ro.bootloader", "(unknown)");
network = android::base::GetProperty("gsm.operator.alpha", "(unknown)");
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index eef3ac7..adaf29e 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -255,7 +255,13 @@
*/
int DumpFile(const std::string& title, const std::string& path);
- // TODO: fields below should be private once refactor is finished
+ // TODO: members below should be private once refactor is finished
+
+ /*
+ * Updates the overall progress of the bugreport generation by the given weight increment.
+ */
+ void UpdateProgress(int delta);
+
// TODO: initialize fields on constructor
// dumpstate id - unique after each device reboot.
@@ -273,18 +279,19 @@
// When set, defines a socket file-descriptor use to report progress to bugreportz.
int controlSocketFd_ = -1;
- // Build type (such as 'user' or 'eng').
- std::string buildType_;
// Full path of the directory where the bugreport files will be written;
std::string bugreportDir_;
private:
+ // Used by GetInstance() only.
+ Dumpstate(bool dryRun = false, const std::string& buildType = "user");
+
// Whether this is a dry run.
bool dryRun_;
- // Used by GetInstance() only.
- Dumpstate(bool dryRun = false);
+ // Build type (such as 'user' or 'eng').
+ std::string buildType_;
};
// for_each_pid_func = void (*)(int, const char*);
@@ -325,9 +332,6 @@
/* sends a broadcast using Activity Manager */
void send_broadcast(const std::string& action, const std::vector<std::string>& args);
-/* updates the overall progress of dumpstate by the given weight increment */
-void update_progress(int weight);
-
/* prints all the system properties */
void print_properties();
diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp
index 27826a1..d2d5b40 100644
--- a/cmds/dumpstate/tests/dumpstate_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_test.cpp
@@ -26,8 +26,13 @@
#include <thread>
#include <android-base/file.h>
+#include <android-base/properties.h>
+#include <android-base/stringprintf.h>
#include <android-base/strings.h>
+#define LOG_TAG "dumpstate"
+#include <cutils/log.h>
+
using ::testing::EndsWith;
using ::testing::IsEmpty;
using ::testing::StrEq;
@@ -46,6 +51,8 @@
public:
void SetUp() {
SetDryRun(false);
+ SetBuildType(android::base::GetProperty("ro.build.type", "(unknown)"));
+ ds.updateProgress_ = false;
}
// Runs a command and capture `stdout` and `stderr`.
@@ -59,6 +66,54 @@
return status;
}
+ void SetDryRun(bool dryRun) {
+ ALOGD("Setting dryRun_ to %s\n", dryRun ? "true" : "false");
+ ds.dryRun_ = dryRun;
+ }
+
+ void SetBuildType(const std::string& buildType) {
+ ALOGD("Setting buildType_ to '%s'\n", buildType.c_str());
+ ds.buildType_ = buildType;
+ }
+
+ bool IsUserBuild() {
+ return "user" == android::base::GetProperty("ro.build.type", "(unknown)");
+ }
+
+ void DropRoot() {
+ drop_root_user();
+ uid_t uid = getuid();
+ ASSERT_EQ(2000, (int)uid);
+ }
+
+ // TODO: remove when progress is set by Binder callbacks.
+ void AssertSystemProperty(const std::string& key, const std::string& expectedValue) {
+ std::string actualValue = android::base::GetProperty(key, "not set");
+ EXPECT_THAT(expectedValue, StrEq(actualValue)) << "invalid value for property " << key;
+ }
+
+ std::string GetProgressMessage(int progress, int weightTotal, int oldWeightTotal = 0) {
+ EXPECT_EQ(progress, ds.progress_) << "invalid progress";
+ EXPECT_EQ(weightTotal, ds.weightTotal_) << "invalid weightTotal";
+
+ AssertSystemProperty(android::base::StringPrintf("dumpstate.%d.progress", getpid()),
+ std::to_string(progress));
+
+ bool maxIncreased = oldWeightTotal > 0;
+
+ std::string adjustmentMessage = "";
+ if (maxIncreased) {
+ AssertSystemProperty(android::base::StringPrintf("dumpstate.%d.max", getpid()),
+ std::to_string(weightTotal));
+ adjustmentMessage = android::base::StringPrintf(
+ "Adjusting total weight from %d to %d\n", oldWeightTotal, weightTotal);
+ }
+
+ return android::base::StringPrintf("%sSetting progress (dumpstate.%d.progress): %d/%d\n",
+ adjustmentMessage.c_str(), getpid(), progress,
+ weightTotal);
+ }
+
// `stdout` and `stderr` from the last command ran.
std::string out, err;
@@ -67,10 +122,6 @@
std::string echoCommand = "/system/bin/echo";
Dumpstate& ds = Dumpstate::GetInstance();
-
- void SetDryRun(bool dryRun) {
- ds.dryRun_ = dryRun;
- }
};
TEST_F(DumpstateTest, RunCommandNoArgs) {
@@ -213,9 +264,80 @@
" --pid --sleep 20' failed: killed by signal 15\n"));
}
-// TODO: add test for other scenarios:
-// - AsRoot()
-// - DropRoot
-// - test progress
+TEST_F(DumpstateTest, RunCommandProgress) {
+ ds.updateProgress_ = true;
+ ds.weightTotal_ = 30;
+
+ EXPECT_EQ(0, RunCommand("", {simpleCommand}, CommandOptions::WithTimeout(20).Build()));
+ std::string progressMessage = GetProgressMessage(20, 30);
+ EXPECT_THAT(out, StrEq("stdout\n"));
+ EXPECT_THAT(err, StrEq("stderr\n" + progressMessage));
+
+ EXPECT_EQ(0, RunCommand("", {simpleCommand}, CommandOptions::WithTimeout(10).Build()));
+ progressMessage = GetProgressMessage(30, 30);
+ EXPECT_THAT(out, StrEq("stdout\n"));
+ EXPECT_THAT(err, StrEq("stderr\n" + progressMessage));
+
+ // Run a command that will increase maximum timeout.
+ EXPECT_EQ(0, RunCommand("", {simpleCommand}, CommandOptions::WithTimeout(1).Build()));
+ progressMessage = GetProgressMessage(31, 36, 30); // 20% increase
+ EXPECT_THAT(out, StrEq("stdout\n"));
+ EXPECT_THAT(err, StrEq("stderr\n" + progressMessage));
+
+ // Make sure command ran while in dryRun is counted.
+ SetDryRun(true);
+ EXPECT_EQ(0, RunCommand("", {simpleCommand}, CommandOptions::WithTimeout(4).Build()));
+ progressMessage = GetProgressMessage(35, 36);
+ EXPECT_THAT(out, IsEmpty());
+ EXPECT_THAT(err, StrEq(progressMessage));
+}
+
+TEST_F(DumpstateTest, RunCommandDropRoot) {
+ // First check root case - only available when running with 'adb root'.
+ uid_t uid = getuid();
+ if (uid == 0) {
+ EXPECT_EQ(0, RunCommand("", {simpleCommand, "--uid"}));
+ EXPECT_THAT(out, StrEq("0\nstdout\n"));
+ EXPECT_THAT(err, StrEq("stderr\n"));
+ return;
+ }
+ // Then drop root.
+
+ EXPECT_EQ(0, RunCommand("", {simpleCommand, "--uid"},
+ CommandOptions::WithTimeout(1).DropRoot().Build()));
+ EXPECT_THAT(out, StrEq("2000\nstdout\n"));
+ EXPECT_THAT(err, StrEq("stderr\n"));
+}
+
+TEST_F(DumpstateTest, RunCommandAsRootUserBuild) {
+ if (!IsUserBuild()) {
+ // Emulates user build if necessarily.
+ SetBuildType("user");
+ }
+
+ DropRoot();
+
+ EXPECT_EQ(0, RunCommand("", {simpleCommand}, CommandOptions::WithTimeout(1).AsRoot().Build()));
+
+ // We don't know the exact path of su, so we just check for the 'root ...' commands
+ EXPECT_THAT(out, StartsWith("Skipping"));
+ EXPECT_THAT(out, EndsWith("root " + simpleCommand + "' on user build.\n"));
+ EXPECT_THAT(err, IsEmpty());
+}
+
+TEST_F(DumpstateTest, RunCommandAsRootNonUserBuild) {
+ if (IsUserBuild()) {
+ ALOGI("Skipping RunCommandAsRootNonUserBuild on user builds\n");
+ return;
+ }
+
+ DropRoot();
+
+ EXPECT_EQ(0, RunCommand("", {simpleCommand, "--uid"},
+ CommandOptions::WithTimeout(1).AsRoot().Build()));
+
+ EXPECT_THAT(out, StrEq("0\nstdout\n"));
+ EXPECT_THAT(err, StrEq("stderr\n"));
+}
// TODO: test DumpFile()
diff --git a/cmds/dumpstate/tests/dumpstate_test_fixture.cpp b/cmds/dumpstate/tests/dumpstate_test_fixture.cpp
index 9af2246..5be4719 100644
--- a/cmds/dumpstate/tests/dumpstate_test_fixture.cpp
+++ b/cmds/dumpstate/tests/dumpstate_test_fixture.cpp
@@ -17,6 +17,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <sys/types.h>
#include <unistd.h>
#define LOG_TAG "dumpstate"
@@ -42,6 +43,8 @@
*
* - If 1st argument is '--pid', it first prints its pid on `stdout`.
*
+ * - If 1st argument is '--uid', it first prints its uid on `stdout`.
+ *
* - If 1st argument is '--crash', it uses ALOGF to crash and returns 666.
*
* - With argument '--exit' 'CODE', returns CODE;
@@ -75,10 +78,14 @@
index++;
fprintf(stdout, "%d\n", getpid());
fflush(stdout);
+ } else if (strcmp(argv[1], "--uid") == 0) {
+ index++;
+ fprintf(stdout, "%d\n", getuid());
+ fflush(stdout);
}
+ // Then the "common" arguments, if any.
if (argc > index + 1) {
- // Then the "common" arguments.
if (strcmp(argv[index], "--sleep") == 0) {
int napTime = atoi(argv[index + 1]);
fprintf(stdout, "stdout line1\n");
diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp
index b347ce8..a9118b1 100644
--- a/cmds/dumpstate/utils.cpp
+++ b/cmds/dumpstate/utils.cpp
@@ -65,6 +65,9 @@
static bool IsDryRun() {
return Dumpstate::GetInstance().IsDryRun();
}
+static void UpdateProgress(int delta) {
+ ds.UpdateProgress(delta);
+}
/* list of native processes to include in the native dumps */
// This matches the /proc/pid/exe link instead of /proc/pid/cmdline.
@@ -159,11 +162,13 @@
return CommandOptions::CommandOptionsBuilder(timeout);
}
-Dumpstate::Dumpstate(bool dryRun) : dryRun_(dryRun) {
+Dumpstate::Dumpstate(bool dryRun, const std::string& buildType)
+ : dryRun_(dryRun), buildType_(buildType) {
}
Dumpstate& Dumpstate::GetInstance() {
- static Dumpstate sSingleton(android::base::GetBoolProperty("dumpstate.dry_run", false));
+ static Dumpstate sSingleton(android::base::GetBoolProperty("dumpstate.dry_run", false),
+ android::base::GetProperty("ro.build.type", "(unknown)"));
return sSingleton;
}
@@ -535,7 +540,7 @@
printf(") ------\n");
}
if (IsDryRun()) {
- update_progress(WEIGHT_FILE);
+ UpdateProgress(WEIGHT_FILE);
close(fd);
return 0;
}
@@ -576,7 +581,7 @@
}
}
}
- update_progress(WEIGHT_FILE);
+ UpdateProgress(WEIGHT_FILE);
close(fd);
if (!newline) printf("\n");
@@ -808,7 +813,7 @@
if (!title.empty()) {
printf("\t(skipped on dry run)\n");
}
- update_progress(options.Timeout());
+ UpdateProgress(options.Timeout());
return 0;
}
@@ -905,7 +910,7 @@
}
if (weight > 0) {
- update_progress(weight);
+ UpdateProgress(weight);
}
return status;
}
@@ -1279,21 +1284,21 @@
}
// TODO: make this function thread safe if sections are generated in parallel.
-void update_progress(int delta) {
- if (!ds.updateProgress_) return;
+void Dumpstate::UpdateProgress(int delta) {
+ if (!updateProgress_) return;
- ds.progress_ += delta;
+ progress_ += delta;
char key[PROPERTY_KEY_MAX];
char value[PROPERTY_VALUE_MAX];
// adjusts max on the fly
- if (ds.progress_ > ds.weightTotal_) {
- int newTotal = ds.weightTotal_ * 1.2;
- MYLOGD("Adjusting total weight from %d to %d\n", ds.weightTotal_, newTotal);
- ds.weightTotal_ = newTotal;
+ if (progress_ > weightTotal_) {
+ int newTotal = weightTotal_ * 1.2;
+ MYLOGD("Adjusting total weight from %d to %d\n", weightTotal_, newTotal);
+ weightTotal_ = newTotal;
snprintf(key, sizeof(key), "dumpstate.%d.max", getpid());
- snprintf(value, sizeof(value), "%d", ds.weightTotal_);
+ snprintf(value, sizeof(value), "%d", weightTotal_);
int status = property_set(key, value);
if (status != 0) {
MYLOGE("Could not update max weight by setting system property %s to %s: %d\n",
@@ -1302,20 +1307,20 @@
}
snprintf(key, sizeof(key), "dumpstate.%d.progress", getpid());
- snprintf(value, sizeof(value), "%d", ds.progress_);
+ snprintf(value, sizeof(value), "%d", progress_);
- if (ds.progress_ % 100 == 0) {
+ if (progress_ % 100 == 0) {
// We don't want to spam logcat, so only log multiples of 100.
- MYLOGD("Setting progress (%s): %s/%d\n", key, value, ds.weightTotal_);
+ MYLOGD("Setting progress (%s): %s/%d\n", key, value, weightTotal_);
} else {
// stderr is ignored on normal invocations, but useful when calling /system/bin/dumpstate
// directly for debuggging.
- fprintf(stderr, "Setting progress (%s): %s/%d\n", key, value, ds.weightTotal_);
+ fprintf(stderr, "Setting progress (%s): %s/%d\n", key, value, weightTotal_);
}
- if (ds.controlSocketFd_ >= 0) {
- dprintf(ds.controlSocketFd_, "PROGRESS:%d/%d\n", ds.progress_, ds.weightTotal_);
- fsync(ds.controlSocketFd_);
+ if (controlSocketFd_ >= 0) {
+ dprintf(controlSocketFd_, "PROGRESS:%d/%d\n", progress_, weightTotal_);
+ fsync(controlSocketFd_);
}
int status = property_set(key, value);