Have setup test use an an output Err
Ths suppresses stdout printing during the test, and allows confirming
the error message as well.
See also:
https://gn-review.googlesource.com/c/gn/+/8940
https://gn-review.googlesource.com/c/gn/+/8500
Change-Id: I1457d7611c8b4327c1cda083f1408ca7cc0cd1f8
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/8941
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
diff --git a/src/gn/setup.cc b/src/gn/setup.cc
index 1d2bea2..c1d8688 100644
--- a/src/gn/setup.cc
+++ b/src/gn/setup.cc
@@ -350,6 +350,19 @@
bool Setup::DoSetup(const std::string& build_dir,
bool force_create,
const base::CommandLine& cmdline) {
+ Err err;
+ if (!DoSetupWithErr(build_dir, force_create, cmdline, &err)) {
+ err.PrintToStdout();
+ return false;
+ }
+ DCHECK(!err.has_error());
+ return true;
+}
+
+bool Setup::DoSetupWithErr(const std::string& build_dir,
+ bool force_create,
+ const base::CommandLine& cmdline,
+ Err* err) {
scheduler_.set_verbose_logging(cmdline.HasSwitch(switches::kVerbose));
if (cmdline.HasSwitch(switches::kTime) ||
cmdline.HasSwitch(switches::kTracelog))
@@ -357,15 +370,15 @@
ScopedTrace setup_trace(TraceItem::TRACE_SETUP, "DoSetup");
- if (!FillSourceDir(cmdline))
+ if (!FillSourceDir(cmdline, err))
return false;
- if (!RunConfigFile())
+ if (!RunConfigFile(err))
return false;
- if (!FillOtherConfig(cmdline))
+ if (!FillOtherConfig(cmdline, err))
return false;
// Must be after FillSourceDir to resolve.
- if (!FillBuildDir(build_dir, !force_create))
+ if (!FillBuildDir(build_dir, !force_create, err))
return false;
// Apply project-specific default (if specified).
@@ -377,16 +390,14 @@
}
if (fill_arguments_) {
- if (!FillArguments(cmdline))
+ if (!FillArguments(cmdline, err))
return false;
}
- if (!FillPythonPath(cmdline))
+ if (!FillPythonPath(cmdline, err))
return false;
// Check for unused variables in the .gn file.
- Err err;
- if (!dotfile_scope_.CheckForUnusedVars(&err)) {
- err.PrintToStdout();
+ if (!dotfile_scope_.CheckForUnusedVars(err)) {
return false;
}
@@ -460,7 +471,7 @@
return true;
}
-bool Setup::FillArguments(const base::CommandLine& cmdline) {
+bool Setup::FillArguments(const base::CommandLine& cmdline, Err* err) {
// Use the args on the command line if specified, and save them. Do this even
// if the list is empty (this means clear any defaults).
// If --args is not set, args.gn file does not exist and gen_empty_args
@@ -471,8 +482,8 @@
auto switch_value = cmdline.GetSwitchValueASCII(switches::kArgs);
if (cmdline.HasSwitch(switches::kArgs) ||
(gen_empty_args_ && !PathExists(build_arg_file))) {
- if (!FillArgsFromCommandLine(switch_value.empty() ? kDefaultArgsGn
- : switch_value)) {
+ if (!FillArgsFromCommandLine(
+ switch_value.empty() ? kDefaultArgsGn : switch_value, err)) {
return false;
}
SaveArgsToFile();
@@ -480,17 +491,17 @@
}
// No command line args given, use the arguments from the build dir (if any).
- return FillArgsFromFile();
+ return FillArgsFromFile(err);
}
-bool Setup::FillArgsFromCommandLine(const std::string& args) {
+bool Setup::FillArgsFromCommandLine(const std::string& args, Err* err) {
args_input_file_ = std::make_unique<InputFile>(SourceFile());
args_input_file_->SetContents(args);
args_input_file_->set_friendly_name("the command-line \"--args\"");
- return FillArgsFromArgsInputFile();
+ return FillArgsFromArgsInputFile(err);
}
-bool Setup::FillArgsFromFile() {
+bool Setup::FillArgsFromFile(Err* err) {
ScopedTrace setup_trace(TraceItem::TRACE_SETUP, "Load args file");
SourceFile build_arg_source_file = GetBuildArgFile();
@@ -514,22 +525,19 @@
"build arg file (use \"gn args <out_dir>\" to edit)");
setup_trace.Done(); // Only want to count the load as part of the trace.
- return FillArgsFromArgsInputFile();
+ return FillArgsFromArgsInputFile(err);
}
-bool Setup::FillArgsFromArgsInputFile() {
+bool Setup::FillArgsFromArgsInputFile(Err* err) {
ScopedTrace setup_trace(TraceItem::TRACE_SETUP, "Parse args");
- Err err;
- args_tokens_ = Tokenizer::Tokenize(args_input_file_.get(), &err);
- if (err.has_error()) {
- err.PrintToStdout();
+ args_tokens_ = Tokenizer::Tokenize(args_input_file_.get(), err);
+ if (err->has_error()) {
return false;
}
- args_root_ = Parser::Parse(args_tokens_, &err);
- if (err.has_error()) {
- err.PrintToStdout();
+ args_root_ = Parser::Parse(args_tokens_, err);
+ if (err->has_error()) {
return false;
}
@@ -538,9 +546,8 @@
SourceDir root_source_dir =
SourceDirForCurrentDirectory(build_settings_.root_path());
arg_scope.set_source_dir(root_source_dir);
- args_root_->Execute(&arg_scope, &err);
- if (err.has_error()) {
- err.PrintToStdout();
+ args_root_->Execute(&arg_scope, err);
+ if (err->has_error()) {
return false;
}
@@ -586,7 +593,7 @@
return true;
}
-bool Setup::FillSourceDir(const base::CommandLine& cmdline) {
+bool Setup::FillSourceDir(const base::CommandLine& cmdline, Err* err) {
// Find the .gn file.
base::FilePath root_path;
@@ -596,10 +603,9 @@
if (!relative_root_path.empty()) {
root_path = base::MakeAbsoluteFilePath(relative_root_path);
if (root_path.empty()) {
- Err(Location(), "Root source path not found.",
- "The path \"" + FilePathToUTF8(relative_root_path) +
- "\" doesn't exist.")
- .PrintToStdout();
+ *err = Err(Location(), "Root source path not found.",
+ "The path \"" + FilePathToUTF8(relative_root_path) +
+ "\" doesn't exist.");
return false;
}
@@ -613,10 +619,9 @@
} else {
dotfile_name_ = base::MakeAbsoluteFilePath(dotfile_path);
if (dotfile_name_.empty()) {
- Err(Location(), "Could not load dotfile.",
- "The file \"" + FilePathToUTF8(dotfile_path) +
- "\" couldn't be loaded.")
- .PrintToStdout();
+ *err = Err(Location(), "Could not load dotfile.",
+ "The file \"" + FilePathToUTF8(dotfile_path) +
+ "\" couldn't be loaded.");
return false;
}
// Only set dotfile_name if it was passed explicitly.
@@ -629,10 +634,9 @@
base::GetCurrentDirectory(&cur_dir);
dotfile_name_ = FindDotFile(cur_dir);
if (dotfile_name_.empty()) {
- Err(Location(), "Can't find source root.",
+ *err = Err(Location(), "Can't find source root.",
"I could not find a \".gn\" file in the current directory or any "
- "parent,\nand the --root command-line argument was not specified.")
- .PrintToStdout();
+ "parent,\nand the --root command-line argument was not specified.");
return false;
}
root_path = dotfile_name_.DirName();
@@ -640,10 +644,9 @@
base::FilePath root_realpath = base::MakeAbsoluteFilePath(root_path);
if (root_realpath.empty()) {
- Err(Location(), "Can't get the real root path.",
- "I could not get the real path of \"" + FilePathToUTF8(root_path) +
- "\".")
- .PrintToStdout();
+ *err = Err(Location(), "Can't get the real root path.",
+ "I could not get the real path of \"" +
+ FilePathToUTF8(root_path) + "\".");
return false;
}
if (scheduler_.verbose_logging())
@@ -653,32 +656,30 @@
return true;
}
-bool Setup::FillBuildDir(const std::string& build_dir, bool require_exists) {
- Err err;
+bool Setup::FillBuildDir(const std::string& build_dir,
+ bool require_exists,
+ Err* err) {
SourceDir resolved =
SourceDirForCurrentDirectory(build_settings_.root_path())
- .ResolveRelativeDir(Value(nullptr, build_dir), &err,
+ .ResolveRelativeDir(Value(nullptr, build_dir), err,
build_settings_.root_path_utf8());
- if (err.has_error()) {
- err.PrintToStdout();
+ if (err->has_error()) {
return false;
}
base::FilePath build_dir_path = build_settings_.GetFullPath(resolved);
if (!base::CreateDirectory(build_dir_path)) {
- Err(Location(), "Can't create the build dir.",
- "I could not create the build dir \"" + FilePathToUTF8(build_dir_path) +
- "\".")
- .PrintToStdout();
+ *err = Err(Location(), "Can't create the build dir.",
+ "I could not create the build dir \"" +
+ FilePathToUTF8(build_dir_path) + "\".");
return false;
}
base::FilePath build_dir_realpath =
base::MakeAbsoluteFilePath(build_dir_path);
if (build_dir_realpath.empty()) {
- Err(Location(), "Can't get the real build dir path.",
- "I could not get the real path of \"" + FilePathToUTF8(build_dir_path) +
- "\".")
- .PrintToStdout();
+ *err = Err(Location(), "Can't get the real build dir path.",
+ "I could not get the real path of \"" +
+ FilePathToUTF8(build_dir_path) + "\".");
return false;
}
resolved = SourceDirForPath(build_settings_.root_path(), build_dir_realpath);
@@ -689,12 +690,12 @@
if (require_exists) {
if (!base::PathExists(
build_dir_path.Append(FILE_PATH_LITERAL("build.ninja")))) {
- Err(Location(), "Not a build directory.",
+ *err = Err(
+ Location(), "Not a build directory.",
"This command requires an existing build directory. I interpreted "
"your input\n\"" +
build_dir + "\" as:\n " + FilePathToUTF8(build_dir_path) +
- "\nwhich doesn't seem to contain a previously-generated build.")
- .PrintToStdout();
+ "\nwhich doesn't seem to contain a previously-generated build.");
return false;
}
}
@@ -703,7 +704,7 @@
return true;
}
-bool Setup::FillPythonPath(const base::CommandLine& cmdline) {
+bool Setup::FillPythonPath(const base::CommandLine& cmdline, Err* err) {
// Trace this since it tends to be a bit slow on Windows.
ScopedTrace setup_trace(TraceItem::TRACE_SETUP, "Fill Python Path");
const Value* value = dotfile_scope_.GetValue("script_executable", true);
@@ -711,9 +712,7 @@
build_settings_.set_python_path(
cmdline.GetSwitchValuePath(switches::kScriptExecutable));
} else if (value) {
- Err err;
- if (!value->VerifyTypeIs(Value::STRING, &err)) {
- err.PrintToStdout();
+ if (!value->VerifyTypeIs(Value::STRING, err)) {
return false;
}
build_settings_.set_python_path(
@@ -735,28 +734,25 @@
return true;
}
-bool Setup::RunConfigFile() {
+bool Setup::RunConfigFile(Err* err) {
if (scheduler_.verbose_logging())
scheduler_.Log("Got dotfile", FilePathToUTF8(dotfile_name_));
dotfile_input_file_ = std::make_unique<InputFile>(SourceFile("//.gn"));
if (!dotfile_input_file_->Load(dotfile_name_)) {
- Err(Location(), "Could not load dotfile.",
- "The file \"" + FilePathToUTF8(dotfile_name_) + "\" couldn't be loaded")
- .PrintToStdout();
+ *err = Err(Location(), "Could not load dotfile.",
+ "The file \"" + FilePathToUTF8(dotfile_name_) +
+ "\" couldn't be loaded");
return false;
}
- Err err;
- dotfile_tokens_ = Tokenizer::Tokenize(dotfile_input_file_.get(), &err);
- if (err.has_error()) {
- err.PrintToStdout();
+ dotfile_tokens_ = Tokenizer::Tokenize(dotfile_input_file_.get(), err);
+ if (err->has_error()) {
return false;
}
- dotfile_root_ = Parser::Parse(dotfile_tokens_, &err);
- if (err.has_error()) {
- err.PrintToStdout();
+ dotfile_root_ = Parser::Parse(dotfile_tokens_, err);
+ if (err->has_error()) {
return false;
}
@@ -767,17 +763,15 @@
// Also add a build dependency to the scope, which is used by `gn analyze`.
dotfile_scope_.AddBuildDependencyFile(SourceFile("//.gn"));
- dotfile_root_->Execute(&dotfile_scope_, &err);
- if (err.has_error()) {
- err.PrintToStdout();
+ dotfile_root_->Execute(&dotfile_scope_, err);
+ if (err->has_error()) {
return false;
}
return true;
}
-bool Setup::FillOtherConfig(const base::CommandLine& cmdline) {
- Err err;
+bool Setup::FillOtherConfig(const base::CommandLine& cmdline, Err* err) {
SourceDir current_dir("//");
Label root_target_label(current_dir, "");
@@ -786,8 +780,7 @@
const Value* secondary_value =
dotfile_scope_.GetValue("secondary_source", true);
if (secondary_value) {
- if (!secondary_value->VerifyTypeIs(Value::STRING, &err)) {
- err.PrintToStdout();
+ if (!secondary_value->VerifyTypeIs(Value::STRING, err)) {
return false;
}
build_settings_.SetSecondarySourcePath(
@@ -798,8 +791,7 @@
const Value* build_file_extension_value =
dotfile_scope_.GetValue("build_file_extension", true);
if (build_file_extension_value) {
- if (!build_file_extension_value->VerifyTypeIs(Value::STRING, &err)) {
- err.PrintToStdout();
+ if (!build_file_extension_value->VerifyTypeIs(Value::STRING, err)) {
return false;
}
@@ -807,8 +799,8 @@
auto normalized_extension = UTF8ToFilePath(extension).value();
if (normalized_extension.find_first_of(base::FilePath::kSeparators) !=
base::FilePath::StringType::npos) {
- Err(Location(), "Build file extension '" + extension + "' cannot " +
- "contain a path separator").PrintToStdout();
+ *err = Err(Location(), "Build file extension '" + extension +
+ "' cannot " + "contain a path separator");
return false;
}
loader_->set_build_file_extension(extension);
@@ -818,8 +810,7 @@
const Value* ninja_required_version_value =
dotfile_scope_.GetValue("ninja_required_version", true);
if (ninja_required_version_value) {
- if (!ninja_required_version_value->VerifyTypeIs(Value::STRING, &err)) {
- err.PrintToStdout();
+ if (!ninja_required_version_value->VerifyTypeIs(Value::STRING, err)) {
return false;
}
std::optional<Version> version =
@@ -836,15 +827,13 @@
// Root build file.
const Value* root_value = dotfile_scope_.GetValue("root", true);
if (root_value) {
- if (!root_value->VerifyTypeIs(Value::STRING, &err)) {
- err.PrintToStdout();
+ if (!root_value->VerifyTypeIs(Value::STRING, err)) {
return false;
}
root_target_label = Label::Resolve(current_dir, std::string_view(), Label(),
- *root_value, &err);
- if (err.has_error()) {
- err.PrintToStdout();
+ *root_value, err);
+ if (err->has_error()) {
return false;
}
}
@@ -863,8 +852,7 @@
"didn't specify a \"buildconfig\" value.")
.PrintToStdout();
return false;
- } else if (!build_config_value->VerifyTypeIs(Value::STRING, &err)) {
- err.PrintToStdout();
+ } else if (!build_config_value->VerifyTypeIs(Value::STRING, err)) {
return false;
}
build_settings_.set_build_config_file(
@@ -876,9 +864,8 @@
if (check_targets_value) {
check_patterns_.reset(new std::vector<LabelPattern>);
ExtractListOfLabelPatterns(&build_settings_, *check_targets_value,
- current_dir, check_patterns_.get(), &err);
- if (err.has_error()) {
- err.PrintToStdout();
+ current_dir, check_patterns_.get(), err);
+ if (err->has_error()) {
return false;
}
}
@@ -886,8 +873,7 @@
const Value* check_system_includes_value =
dotfile_scope_.GetValue("check_system_includes", true);
if (check_system_includes_value) {
- if (!check_system_includes_value->VerifyTypeIs(Value::BOOLEAN, &err)) {
- err.PrintToStdout();
+ if (!check_system_includes_value->VerifyTypeIs(Value::BOOLEAN, err)) {
return false;
}
check_system_includes_ = check_system_includes_value->boolean_value();
@@ -898,20 +884,17 @@
dotfile_scope_.GetValue("exec_script_whitelist", true);
if (exec_script_whitelist_value) {
// Fill the list of targets to check.
- if (!exec_script_whitelist_value->VerifyTypeIs(Value::LIST, &err)) {
- err.PrintToStdout();
+ if (!exec_script_whitelist_value->VerifyTypeIs(Value::LIST, err)) {
return false;
}
std::unique_ptr<SourceFileSet> whitelist =
std::make_unique<SourceFileSet>();
for (const auto& item : exec_script_whitelist_value->list_value()) {
- if (!item.VerifyTypeIs(Value::STRING, &err)) {
- err.PrintToStdout();
+ if (!item.VerifyTypeIs(Value::STRING, err)) {
return false;
}
- whitelist->insert(current_dir.ResolveRelativeFile(item, &err));
- if (err.has_error()) {
- err.PrintToStdout();
+ whitelist->insert(current_dir.ResolveRelativeFile(item, err));
+ if (err->has_error()) {
return false;
}
}
@@ -922,8 +905,7 @@
const Value* default_args_value =
dotfile_scope_.GetValue("default_args", true);
if (default_args_value) {
- if (!default_args_value->VerifyTypeIs(Value::SCOPE, &err)) {
- err.PrintToStdout();
+ if (!default_args_value->VerifyTypeIs(Value::SCOPE, err)) {
return false;
}
@@ -933,8 +915,7 @@
const Value* arg_file_template_value =
dotfile_scope_.GetValue("arg_file_template", true);
if (arg_file_template_value) {
- if (!arg_file_template_value->VerifyTypeIs(Value::STRING, &err)) {
- err.PrintToStdout();
+ if (!arg_file_template_value->VerifyTypeIs(Value::STRING, err)) {
return false;
}
SourceFile path(arg_file_template_value->string_value());
diff --git a/src/gn/setup.h b/src/gn/setup.h
index 25389f1..bf1e1ca 100644
--- a/src/gn/setup.h
+++ b/src/gn/setup.h
@@ -57,6 +57,12 @@
bool force_create,
const base::CommandLine& cmdline);
+ // Same as DoSetup() but used for tests to capture error output.
+ bool DoSetupWithErr(const std::string& build_dir,
+ bool force_create,
+ const base::CommandLine& cmdline,
+ Err* err);
+
// Runs the load, returning true on success. On failure, prints the error
// and returns false. This includes both RunPreMessageLoop() and
// RunPostMessageLoop().
@@ -117,35 +123,38 @@
bool RunPostMessageLoop(const base::CommandLine& cmdline);
// Fills build arguments. Returns true on success.
- bool FillArguments(const base::CommandLine& cmdline);
+ bool FillArguments(const base::CommandLine& cmdline, Err* err);
// Fills the build arguments from the command line or from the build arg file.
- bool FillArgsFromCommandLine(const std::string& args);
- bool FillArgsFromFile();
+ bool FillArgsFromCommandLine(const std::string& args, Err* err);
+ bool FillArgsFromFile(Err* err);
// Given an already-loaded args_input_file_, parses and saves the resulting
// arguments. Backend for the different FillArgs variants.
- bool FillArgsFromArgsInputFile();
+ bool FillArgsFromArgsInputFile(Err* err);
// Writes the build arguments to the build arg file.
bool SaveArgsToFile();
- // Fills the root directory into the settings. Returns true on success.
- bool FillSourceDir(const base::CommandLine& cmdline);
+ // Fills the root directory into the settings. Returns true on success, or
+ // |err| filled out.
+ bool FillSourceDir(const base::CommandLine& cmdline, Err* err);
// Fills the build directory given the value the user has specified.
// Must happen after FillSourceDir so we can resolve source-relative
// paths. If require_exists is false, it will fail if the dir doesn't exist.
- bool FillBuildDir(const std::string& build_dir, bool require_exists);
+ bool FillBuildDir(const std::string& build_dir,
+ bool require_exists,
+ Err* err);
// Fills the python path portion of the command line. On failure, sets
// it to just "python".
- bool FillPythonPath(const base::CommandLine& cmdline);
+ bool FillPythonPath(const base::CommandLine& cmdline, Err* err);
// Run config file.
- bool RunConfigFile();
+ bool RunConfigFile(Err* err);
- bool FillOtherConfig(const base::CommandLine& cmdline);
+ bool FillOtherConfig(const base::CommandLine& cmdline, Err* err);
BuildSettings build_settings_;
scoped_refptr<LoaderImpl> loader_;
diff --git a/src/gn/setup_unittest.cc b/src/gn/setup_unittest.cc
index eda66ab..af10a6e 100644
--- a/src/gn/setup_unittest.cc
+++ b/src/gn/setup_unittest.cc
@@ -11,6 +11,7 @@
#include "gn/filesystem_utils.h"
#include "gn/switches.h"
#include "gn/test_with_scheduler.h"
+#include "util/build_config.h"
using SetupTest = TestWithScheduler;
@@ -45,7 +46,9 @@
EXPECT_EQ(gen_deps[0], base::MakeAbsoluteFilePath(dot_gn_name));
}
-static void RunExtensionCheckTest(std::string extension, bool success) {
+static void RunExtensionCheckTest(std::string extension,
+ bool success,
+ const std::string& expected_error_message) {
base::CommandLine cmdline(base::CommandLine::NO_PROGRAM);
// Create a temp directory containing a .gn file and a BUILDCONFIG.gn file,
@@ -54,8 +57,10 @@
ASSERT_TRUE(in_temp_dir.CreateUniqueTempDir());
base::FilePath in_path = in_temp_dir.GetPath();
base::FilePath dot_gn_name = in_path.Append(FILE_PATH_LITERAL(".gn"));
- WriteFile(dot_gn_name, "buildconfig = \"//BUILDCONFIG.gn\"\n\
- build_file_extension = \"" + extension + "\"");
+ WriteFile(dot_gn_name,
+ "buildconfig = \"//BUILDCONFIG.gn\"\n\
+ build_file_extension = \"" +
+ extension + "\"");
WriteFile(in_path.Append(FILE_PATH_LITERAL("BUILDCONFIG.gn")), "");
cmdline.AppendSwitchASCII(switches::kRoot, FilePathToUTF8(in_path));
@@ -65,16 +70,25 @@
// Run setup and check that its status.
Setup setup;
+ Err err;
EXPECT_EQ(success,
- setup.DoSetup(FilePathToUTF8(build_temp_dir.GetPath()), true, cmdline));
+ setup.DoSetupWithErr(FilePathToUTF8(build_temp_dir.GetPath()), true,
+ cmdline, &err));
+ EXPECT_EQ(success, !err.has_error());
+ EXPECT_EQ(expected_error_message, err.message());
}
TEST_F(SetupTest, NoSeparatorInExtension) {
RunExtensionCheckTest(
- "hello" + std::string(1, base::FilePath::kSeparators[0]) + "world",
- false);
+ "hello" + std::string(1, base::FilePath::kSeparators[0]) + "world", false,
+#if defined(OS_WIN)
+ "Build file extension 'hello\\world' cannot contain a path separator"
+#else
+ "Build file extension 'hello/world' cannot contain a path separator"
+#endif
+ );
}
TEST_F(SetupTest, Extension) {
- RunExtensionCheckTest("yay", true);
+ RunExtensionCheckTest("yay", true, "");
}