[Lldb-commits] [lldb] r282171 - added environment variable-related Args gtests
Todd Fiala via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 22 09:50:33 PDT 2016
Okay. Yeah I don't see us needing to support the equal with trailing nothing.
-Todd
> On Sep 22, 2016, at 9:23 AM, Zachary Turner <zturner at google.com> wrote:
>
> 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/4b4c7866/attachment-0001.html>
More information about the lldb-commits
mailing list