[Lldb-commits] [lldb] r282171 - added environment variable-related Args gtests
Zachary Turner via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 22 09:23:42 PDT 2016
yea I mostly just wanted to know if we needed to specificlaly distinguish
between ARG=\0 and ARG\0. Because if so the second parameter would need to
be Optional<StringRef> since StringRef has no way to differentiate between
"I don't refer to anything" versus "I refer to the empty string".
(Technically it kinda does, but I think it relies on an implementation
detail so shouldn't be used)
On Thu, Sep 22, 2016 at 9:20 AM Todd Fiala <todd.fiala at gmail.com> wrote:
> Primarily because we pass them verbatim to posix_spawn and other
> launchers, and there it is legitimate to not have an equal with trailing
> nothingness. On the Xcode side, we use a ton of environment variables.
>
> As to whether there is a difference between ARG1=\0 and ARG1\0, I'm not
> sure.
>
> On Thu, Sep 22, 2016 at 9:12 AM, Zachary Turner <zturner at google.com>
> wrote:
>
> Thanks for the test. Is there any practical difference between "ARGS="
> and "ARGS"?
>
> On Thu, Sep 22, 2016 at 9:08 AM Todd Fiala via lldb-commits <
> lldb-commits at lists.llvm.org> wrote:
>
> Author: tfiala
> Date: Thu Sep 22 11:00:01 2016
> New Revision: 282171
>
> URL: http://llvm.org/viewvc/llvm-project?rev=282171&view=rev
> Log:
> added environment variable-related Args gtests
>
> Also fixed up a couple misbehaving functions. It is perfectly
> legal to have env vars with no values (i.e. the '=' and following
> need not be present).
>
> Modified:
> lldb/trunk/source/Interpreter/Args.cpp
> lldb/trunk/unittests/Interpreter/TestArgs.cpp
>
> Modified: lldb/trunk/source/Interpreter/Args.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Args.cpp?rev=282171&r1=282170&r2=282171&view=diff
>
> ==============================================================================
> --- lldb/trunk/source/Interpreter/Args.cpp (original)
> +++ lldb/trunk/source/Interpreter/Args.cpp Thu Sep 22 11:00:01 2016
> @@ -976,13 +976,15 @@ void Args::LongestCommonPrefix(std::stri
>
> void Args::AddOrReplaceEnvironmentVariable(llvm::StringRef env_var_name,
> llvm::StringRef new_value) {
> - if (env_var_name.empty() || new_value.empty())
> + if (env_var_name.empty())
> return;
>
> // Build the new entry.
> std::string var_string(env_var_name);
> - var_string += "=";
> - var_string += new_value;
> + if (!new_value.empty()) {
> + var_string += "=";
> + var_string += new_value;
> + }
>
> size_t index = 0;
> if (ContainsEnvironmentVariable(env_var_name, &index)) {
> @@ -1006,7 +1008,7 @@ bool Args::ContainsEnvironmentVariable(l
>
> llvm::StringRef name, value;
> std::tie(name, value) = arg_value.split('=');
> - if (name == env_var_name && !value.empty()) {
> + if (name == env_var_name) {
> if (argument_index)
> *argument_index = i;
> return true;
>
> Modified: lldb/trunk/unittests/Interpreter/TestArgs.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Interpreter/TestArgs.cpp?rev=282171&r1=282170&r2=282171&view=diff
>
> ==============================================================================
> --- lldb/trunk/unittests/Interpreter/TestArgs.cpp (original)
> +++ lldb/trunk/unittests/Interpreter/TestArgs.cpp Thu Sep 22 11:00:01 2016
> @@ -11,6 +11,9 @@
>
> #include "lldb/Interpreter/Args.h"
>
> +#include <limits>
> +#include <sstream>
> +
> using namespace lldb_private;
>
> TEST(ArgsTest, TestSingleArg) {
> @@ -153,3 +156,85 @@ TEST(ArgsTest, StringToScriptLanguage) {
> }
>
> TEST(ArgsTest, StringToVersion) {}
> +
> +// Environment Variable Tests
> +
> +class EnvVarFixture: public ::testing::Test {
> +protected:
> +
> + void SetUp() {
> + args.AppendArgument(llvm::StringRef("Arg1=foo"));
> + args.AppendArgument(llvm::StringRef("Arg2"));
> + args.AppendArgument(llvm::StringRef("Arg3=bar"));
> + }
> +
> + size_t GetIndexForEnvVar(llvm::StringRef envvar_name) {
> + size_t argument_index = std::numeric_limits<size_t>::max();
> + EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name,
> + &argument_index));
> + EXPECT_LT(argument_index, args.GetArgumentCount());
> + return argument_index;
> + }
> +
> + Args args;
> +};
> +
> +
> +TEST_F(EnvVarFixture, TestContainsEnvironmentVariableNoValue) {
> +
> EXPECT_TRUE(args.ContainsEnvironmentVariable(llvm::StringRef("Arg2")));
> +}
> +
> +TEST_F(EnvVarFixture, TestContainsEnvironmentVariableWithValue) {
> +
> EXPECT_TRUE(args.ContainsEnvironmentVariable(llvm::StringRef("Arg3")));
> +}
> +
> +TEST_F(EnvVarFixture, TestContainsEnvironmentVariableNonExistentVariable)
> {
> + auto nonexistent_envvar = llvm::StringRef("ThisEnvVarShouldNotExist");
> + EXPECT_FALSE(args.ContainsEnvironmentVariable(nonexistent_envvar));
> +}
> +
> +TEST_F(EnvVarFixture,
> TestReplaceEnvironmentVariableInitialNoValueWithNoValue) {
> + auto envvar_name = llvm::StringRef("Arg2");
> + auto argument_index = GetIndexForEnvVar(envvar_name);
> +
> + args.AddOrReplaceEnvironmentVariable(envvar_name,
> llvm::StringRef(""));
> + EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name));
> + EXPECT_EQ(envvar_name, args.GetArgumentAtIndex(argument_index));
> +}
> +
> +TEST_F(EnvVarFixture,
> TestReplaceEnvironmentVariableInitialNoValueWithValue) {
> + auto envvar_name = llvm::StringRef("Arg2");
> + auto argument_index = GetIndexForEnvVar(envvar_name);
> +
> + auto new_value = llvm::StringRef("NewValue");
> + args.AddOrReplaceEnvironmentVariable(envvar_name, new_value);
> + EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name));
> +
> + std::stringstream stream;
> + stream << envvar_name.str() << '=' << new_value.str();
> + EXPECT_EQ(llvm::StringRef(stream.str()),
> + args.GetArgumentAtIndex(argument_index));
> +}
> +
> +TEST_F(EnvVarFixture,
> TestReplaceEnvironmentVariableInitialValueWithNoValue) {
> + auto envvar_name = llvm::StringRef("Arg1");
> + auto argument_index = GetIndexForEnvVar(envvar_name);
> +
> + args.AddOrReplaceEnvironmentVariable(envvar_name,
> llvm::StringRef(""));
> + EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name));
> + EXPECT_EQ(envvar_name, args.GetArgumentAtIndex(argument_index));
> +}
> +
> +TEST_F(EnvVarFixture,
> TestReplaceEnvironmentVariableInitialValueWithValue) {
> + auto envvar_name = llvm::StringRef("Arg1");
> + auto argument_index = GetIndexForEnvVar(envvar_name);
> +
> + auto new_value = llvm::StringRef("NewValue");
> + args.AddOrReplaceEnvironmentVariable(envvar_name, new_value);
> + EXPECT_TRUE(args.ContainsEnvironmentVariable(envvar_name));
> +
> + std::stringstream stream;
> + stream << envvar_name.str() << '=' << new_value.str();
> + EXPECT_EQ(llvm::StringRef(stream.str()),
> + args.GetArgumentAtIndex(argument_index));
> +}
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
>
>
> --
> -Todd
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160922/8f0f3c61/attachment.html>
More information about the lldb-commits
mailing list