Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(400)

Unified Diff: net/test/spawned_test_server/local_test_server_win.cc

Issue 21537002: Remove hard dependency on bundled python_26 in tests (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@glyphcache_20130604
Patch Set: Addressed some review issues. Created 7 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/test/python_utils.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/test/spawned_test_server/local_test_server_win.cc
diff --git a/net/test/spawned_test_server/local_test_server_win.cc b/net/test/spawned_test_server/local_test_server_win.cc
index fd26483ed477b37698538f1f2b609e7d79b343f2..682403fb4a20893015ccb166597456238d554c3b 100644
--- a/net/test/spawned_test_server/local_test_server_win.cc
+++ b/net/test/spawned_test_server/local_test_server_win.cc
@@ -10,6 +10,7 @@
#include "base/base_paths.h"
#include "base/bind.h"
#include "base/command_line.h"
+#include "base/environment.h"
#include "base/files/file_path.h"
#include "base/message_loop/message_loop.h"
#include "base/path_service.h"
@@ -85,6 +86,54 @@ bool ReadData(HANDLE read_fd, HANDLE write_fd,
namespace net {
+class AddedPythonPath {
Paweł Hajdan Jr. 2013/08/22 18:11:53 nit: Rename to ScopedPath, and move to anonymous n
Daniel Bratell 2013/08/23 17:42:30 Won't ScopedPath feel like a strange name for some
Daniel Bratell 2013/08/23 17:43:35 Ignore this. I wrote this comment before reading y
+public:
Paweł Hajdan Jr. 2013/08/22 18:11:53 nit: Indent +1.
+ AddedPythonPath();
+ ~AddedPythonPath();
+
+private:
Paweł Hajdan Jr. 2013/08/22 18:11:53 nit: Indent +1.
+ std::string old_path_;
Paweł Hajdan Jr. 2013/08/22 18:11:53 nit: Please comment all the members.
+ scoped_ptr<base::Environment> environment_;
+ bool path_modified_;
+
+ DISALLOW_COPY_AND_ASSIGN(AddedPythonPath);
+};
+
+AddedPythonPath::AddedPythonPath()
+ : environment_(base::Environment::Create()),
+ path_modified_(false) {
+ // Retrieves the old path, adds third_party/python26 to the end of it and
+ // then restores the original path in the destructor.
+
+ ignore_result(environment_->GetVar("PATH", &old_path_));
+
+ std::string new_value = old_path_;
+ if (!new_value.empty())
+ new_value += ";"; // Path seperator.
Paweł Hajdan Jr. 2013/08/22 18:11:53 nit: No need for the comment (by the way, we'd use
+
+ // Add new path to the end so that the system python is used if available.
+ base::FilePath python_path;
+ if (!PathService::Get(base::DIR_SOURCE_ROOT, &python_path))
Paweł Hajdan Jr. 2013/08/22 18:11:53 Please propagate this error as said before...
+ return;
+ python_path = python_path.AppendASCII("third_party")
Paweł Hajdan Jr. 2013/08/22 18:11:53 Actually it'd be better to pass this as a paramete
+ .AppendASCII("python_26");
Paweł Hajdan Jr. 2013/08/22 18:11:53 nit: This is a bit weird wrapping - if it doesn't
Daniel Bratell 2013/08/23 17:42:30 It was the way the code looked before so I just as
+ new_value += WideToUTF8(python_path.value());
+
+ path_modified_ = environment_->SetVar("PATH", new_value);
+}
+
+AddedPythonPath::~AddedPythonPath() {
+ if (path_modified_) {
+ // Ignoring return values since there is nothing we can do to fix
+ // any problem and leaving a test python at the end of the path is
+ // unlikely to do any harm.
+ if (old_path_.empty())
+ ignore_result(environment_->UnSetVar("PATH"));
+ else
+ ignore_result(environment_->SetVar("PATH", old_path_));
+ }
+}
+
bool LocalTestServer::LaunchPython(const base::FilePath& testserver_path) {
CommandLine python_command(CommandLine::NO_PROGRAM);
if (!GetPythonCommand(&python_command))
@@ -134,6 +183,7 @@ bool LocalTestServer::LaunchPython(const base::FilePath& testserver_path) {
return false;
}
+ AddedPythonPath python_path;
base::LaunchOptions launch_options;
launch_options.inherit_handles = true;
launch_options.job_handle = job_handle_.Get();
« no previous file with comments | « net/test/python_utils.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698