[Lldb-commits] [lldb] r282171 - added environment variable-related Args gtests

[Todd Fiala via lldb-commits]

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/5b6f159b/attachment-0001.html>