update_engine: Use libchromeos to launch subprocesses.
The Subprocess class handles the execution of suprocesses in the
update_engine such as the post-install script and bspatch operations.
This patch migrates this class from using glib functions to use
libchromeos classes with equivalent functionality.
Callsites and unittests were updated to match the new interface.
BUG=chromium:499886
TEST=Unittest still pass. Deployed on link and cros flash another image
using a delta payload.
Change-Id: Ia64d39734e220675113f393a6049e9a9b0fe8409
Reviewed-on: https://chromium-review.googlesource.com/288837
Trybot-Ready: Alex Deymo <[email protected]>
Tested-by: Alex Deymo <[email protected]>
Reviewed-by: Alex Vakulenko <[email protected]>
Commit-Queue: Alex Deymo <[email protected]>
diff --git a/subprocess_unittest.cc b/subprocess_unittest.cc
index dba9e6c..eb95133 100644
--- a/subprocess_unittest.cc
+++ b/subprocess_unittest.cc
@@ -13,6 +13,7 @@
#include <sys/types.h>
#include <unistd.h>
+#include <set>
#include <string>
#include <vector>
@@ -25,7 +26,7 @@
#include <chromeos/message_loops/glib_message_loop.h>
#include <chromeos/message_loops/message_loop.h>
#include <chromeos/message_loops/message_loop_utils.h>
-#include <glib.h>
+#include <chromeos/strings/string_utils.h>
#include <gtest/gtest.h>
#include "update_engine/test_utils.h"
@@ -42,69 +43,83 @@
protected:
void SetUp() override {
loop_.SetAsCurrent();
- }
-
- void TearDown() override {
- EXPECT_EQ(0, chromeos::MessageLoopRunMaxIterations(&loop_, 1));
+ subprocess_.Init();
}
// TODO(deymo): Replace this with a FakeMessageLoop. Subprocess uses glib to
// asynchronously spawn a process, so we need to run a GlibMessageLoop here.
chromeos::GlibMessageLoop loop_;
+ Subprocess subprocess_;
};
namespace {
+
int local_server_port = 0;
-void Callback(int return_code, const string& output, void* /* unused */) {
- EXPECT_EQ(1, return_code);
+void ExpectedResults(int expected_return_code, const string& expected_output,
+ int return_code, const string& output) {
+ EXPECT_EQ(expected_return_code, return_code);
+ EXPECT_EQ(expected_output, output);
MessageLoop::current()->BreakLoop();
}
-void CallbackEcho(int return_code, const string& output, void* /* unused */) {
+void ExpectedEnvVars(int return_code, const string& output) {
EXPECT_EQ(0, return_code);
- EXPECT_NE(string::npos, output.find("this is stdout"));
- EXPECT_NE(string::npos, output.find("this is stderr"));
- MessageLoop::current()->BreakLoop();
-}
-
-void CallbackStdoutOnlyEcho(int return_code,
- const string& output,
- void* /* unused */) {
- EXPECT_EQ(0, return_code);
- EXPECT_NE(string::npos, output.find("on stdout"));
- EXPECT_EQ(string::npos, output.find("on stderr"));
+ const std::set<string> allowed_envs = {"LD_LIBRARY_PATH", "PATH"};
+ for (string key_value : chromeos::string_utils::Split(output, "\n")) {
+ auto key_value_pair = chromeos::string_utils::SplitAtFirst(
+ key_value, "=", true);
+ EXPECT_NE(allowed_envs.end(), allowed_envs.find(key_value_pair.first));
+ }
MessageLoop::current()->BreakLoop();
}
} // namespace
+TEST_F(SubprocessTest, IsASingleton) {
+ EXPECT_EQ(&subprocess_, &Subprocess::Get());
+}
+
+TEST_F(SubprocessTest, InactiveInstancesDontChangeTheSingleton) {
+ std::unique_ptr<Subprocess> another_subprocess(new Subprocess());
+ EXPECT_EQ(&subprocess_, &Subprocess::Get());
+ another_subprocess.reset();
+ EXPECT_EQ(&subprocess_, &Subprocess::Get());
+}
+
TEST_F(SubprocessTest, SimpleTest) {
- Subprocess::Get().Exec(vector<string>{"/bin/false"}, Callback, nullptr);
+ EXPECT_TRUE(subprocess_.Exec({"/bin/false"},
+ base::Bind(&ExpectedResults, 1, "")));
loop_.Run();
}
TEST_F(SubprocessTest, EchoTest) {
- Subprocess::Get().Exec(
- vector<string>{
- "/bin/sh",
- "-c",
- "echo this is stdout; echo this is stderr > /dev/stderr"},
- CallbackEcho,
- nullptr);
+ EXPECT_TRUE(subprocess_.Exec(
+ {"/bin/sh", "-c", "echo this is stdout; echo this is stderr >&2"},
+ base::Bind(&ExpectedResults, 0, "this is stdout\nthis is stderr\n")));
loop_.Run();
}
TEST_F(SubprocessTest, StderrNotIncludedInOutputTest) {
- Subprocess::Get().ExecFlags(
- vector<string>{"/bin/sh", "-c", "echo on stdout; echo on stderr >&2"},
- static_cast<GSpawnFlags>(0),
- false, // don't redirect stderr
- CallbackStdoutOnlyEcho,
- nullptr);
+ EXPECT_TRUE(subprocess_.ExecFlags(
+ {"/bin/sh", "-c", "echo on stdout; echo on stderr >&2"},
+ 0,
+ base::Bind(&ExpectedResults, 0, "on stdout\n")));
loop_.Run();
}
+TEST_F(SubprocessTest, EnvVarsAreFiltered) {
+ EXPECT_TRUE(subprocess_.Exec({"/usr/bin/env"}, base::Bind(&ExpectedEnvVars)));
+ loop_.Run();
+}
+
+TEST_F(SubprocessTest, SynchronousTrueSearchsOnPath) {
+ int rc = -1;
+ EXPECT_TRUE(Subprocess::SynchronousExecFlags(
+ {"true"}, Subprocess::kSearchPath, &rc, nullptr));
+ EXPECT_EQ(0, rc);
+}
+
TEST_F(SubprocessTest, SynchronousEchoTest) {
vector<string> cmd = {
"/bin/sh",
@@ -118,15 +133,16 @@
}
TEST_F(SubprocessTest, SynchronousEchoNoOutputTest) {
- vector<string> cmd = {"/bin/sh", "-c", "echo test"};
int rc = -1;
- ASSERT_TRUE(Subprocess::SynchronousExec(cmd, &rc, nullptr));
+ ASSERT_TRUE(Subprocess::SynchronousExec(
+ {"/bin/sh", "-c", "echo test"},
+ &rc, nullptr));
EXPECT_EQ(0, rc);
}
namespace {
-void CallbackBad(int return_code, const string& output, void* p) {
- CHECK(false) << "should never be called.";
+void CallbackBad(int return_code, const string& output) {
+ ADD_FAILURE() << "should never be called.";
}
// TODO(garnold) this test method uses test_http_server as a representative for
@@ -148,7 +164,7 @@
vector<string> cmd;
cmd.push_back("./test_http_server");
cmd.push_back(temp_file_name);
- uint32_t tag = Subprocess::Get().Exec(cmd, CallbackBad, nullptr);
+ uint32_t tag = Subprocess::Get().Exec(cmd, base::Bind(&CallbackBad));
EXPECT_NE(0, tag);
*spawned = true;
printf("test http server spawned\n");
