Skip to content

Commit

Permalink
[ign-common3] Miscellaneous Cleanups (#107)
Browse files Browse the repository at this point in the history
* Use variable substitution to prevent forward port issues
* Add string Join functionality
* Replace all uses of env with Ignition functions

Signed-off-by: Michael Carroll <[email protected]>
  • Loading branch information
mjcarroll committed Oct 27, 2020
1 parent 77eab3e commit dfc3f10
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 75 deletions.
11 changes: 6 additions & 5 deletions examples/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
cmake_minimum_required(VERSION 2.8 FATAL_ERROR)

# Find the ignition-common library
find_package(ignition-common4 QUIET REQUIRED COMPONENTS events profiler)
set(IGN_COMMON_VER 4)
find_package(ignition-common${IGN_COMMON_VER} QUIET REQUIRED COMPONENTS events profiler)

add_executable(assert_example assert_example.cc)
target_link_libraries(assert_example ignition-common4::core)
target_link_libraries(assert_example ignition-common${IGN_COMMON_VER}::core)

add_executable(console_example console.cc)
target_link_libraries(console_example ignition-common4::core)
target_link_libraries(console_example ignition-common${IGN_COMMON_VER}::core)

add_executable(events_example events.cc)
target_link_libraries(events_example ignition-common4::events)
target_link_libraries(events_example ignition-common${IGN_COMMON_VER}::events)

add_executable(profiler_example profiler.cc)
target_link_libraries(profiler_example ignition-common4::profiler)
target_link_libraries(profiler_example ignition-common${IGN_COMMON_VER}::profiler)
target_compile_definitions(profiler_example PUBLIC "IGN_PROFILER_ENABLE=1")
24 changes: 24 additions & 0 deletions include/ignition/common/StringUtils.hh
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,30 @@ namespace ignition
std::vector<std::string> IGNITION_COMMON_VISIBLE Split(
const std::string &_orig, char _delim);

/// \brief Join a sequence of strings with a delimiter
///
/// Note that this will skip any empty entries in the vector,
/// and will also not prepend or append the delimiter to the
/// final output string
///
/// \param[in] _orig The input sequence of strings
/// \param[in] _delim a string delimiter to join the string with
/// \returns a single string composed of strings joined with the delimiter
std::string IGNITION_COMMON_VISIBLE Join(
const std::vector<std::string> &_orig, const std::string &_delim);

/// \brief Join a sequence of strings with a delimiter
///
/// Note that this will skip any empty entries in the vector,
/// and will also not prepend or append the delimiter to the
/// final output string
///
/// \param[in] _orig The input sequence of strings
/// \param[in] _delim a character to join the string with
/// \returns a single string composed of strings joined with the delimiter
std::string IGNITION_COMMON_VISIBLE Join(
const std::vector<std::string> &_orig, char _delim);

/// \brief return true if string starts with another string
/// \param[in] _s1 the string to check
/// \param[in] _s2 the possible prefix
Expand Down
10 changes: 4 additions & 6 deletions src/Filesystem_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,17 @@
#include <sys/types.h>
#include <unistd.h>

#include "ignition/common/Util.hh"

// The symlink tests should always work on UNIX systems
#define BUILD_SYMLINK_TESTS

/////////////////////////////////////////////////
bool create_and_switch_to_temp_dir(std::string &_new_temp_path)
{
std::string tmppath;
const char *tmp = getenv("TMPDIR");
if (tmp)
{
tmppath = std::string(tmp);
}
else

if (!ignition::common::env("TMPDIR", tmppath))
{
tmppath = std::string("/tmp");
}
Expand Down
24 changes: 24 additions & 0 deletions src/StringUtils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,30 @@ namespace ignition
return pieces;
}

//////////////////////////////////////////////////
std::string Join(const std::vector<std::string> &_orig,
const std::string &_delim)
{
std::string ret = "";
for (size_t ii = 0; ii < _orig.size(); ++ii)
{
if (_orig[ii].empty())
continue;

ret += _orig[ii];
if (ii < _orig.size() - 1)
{
ret += _delim;
}
}
return ret;
}
//////////////////////////////////////////////////
std::string Join(const std::vector<std::string> &_orig, char _delim)
{
return Join(_orig, std::string(1, _delim));
}

//////////////////////////////////////////////////
bool StartsWith(const std::string &_s1, const std::string &_s2)
{
Expand Down
53 changes: 53 additions & 0 deletions src/StringUtils_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,59 @@ TEST(StringUtils, SplitOneDelimiterAtEnd)
EXPECT_EQ("", pieces[1]);
}

/////////////////////////////////////////////////
TEST(Join, Simple)
{
{
std::string delim = " ";
auto pieces = std::vector<std::string>{"Hello", "World"};
auto joined = common::Join(pieces, delim);
EXPECT_EQ("Hello World", joined);
}

{
std::string delim = "x";
auto pieces = std::vector<std::string>{"Hello", "World"};
auto joined = common::Join(pieces, delim);
EXPECT_EQ("HelloxWorld", joined);
}

{
std::string delim = "x";
auto pieces = std::vector<std::string>{
"foo", "bar", "baz", "bing"};
auto joined = common::Join(pieces, delim);
EXPECT_EQ("fooxbarxbazxbing", joined);
}
}

/////////////////////////////////////////////////
TEST(Join, EmptyDelim)
{
std::string delim = "";
auto pieces = std::vector<std::string>{"Hello", "World"};
auto joined = common::Join(pieces, delim);
EXPECT_EQ("HelloWorld", joined);
}

/////////////////////////////////////////////////
TEST(Join, EmptyPart)
{
std::string delim = " ";
auto pieces = std::vector<std::string>{"Hello", "", "World"};
auto joined = common::Join(pieces, delim);
EXPECT_EQ("Hello World", joined);
}

/////////////////////////////////////////////////
TEST(Join, EmptyParts)
{
std::string delim = "x";
auto pieces = std::vector<std::string>{};
auto joined = common::Join(pieces, delim);
EXPECT_EQ("", joined);
}

/////////////////////////////////////////////////
TEST(StartsWith, NotInString)
{
Expand Down
97 changes: 33 additions & 64 deletions src/SystemPaths_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <cstdio>

#include "ignition/common/Util.hh"
#include "ignition/common/StringUtils.hh"
#include "ignition/common/SystemPaths.hh"

#ifdef _WIN32
Expand All @@ -32,24 +33,19 @@

using namespace ignition;

const char kPluginPath[] = "IGN_PLUGIN_PATH";
const char kFilePath[] = "IGN_FILE_PATH";

class SystemPathsFixture : public ::testing::Test
{
// Documentation inherited
public: virtual void SetUp()
{
this->backupPluginPath = "IGN_PLUGIN_PATH=";

if (getenv("IGN_PLUGIN_PATH"))
this->backupPluginPath += getenv("IGN_PLUGIN_PATH");

putenv(const_cast<char*>("IGN_PLUGIN_PATH="));

this->backupFilePath = "IGN_FILE_PATH=";
common::env(kPluginPath, this->backupPluginPath);
common::unsetenv(kPluginPath);

if (getenv("IGN_FILE_PATH"))
this->backupFilePath += getenv("IGN_FILE_PATH");

putenv(const_cast<char*>("IGN_FILE_PATH="));
common::env(kFilePath, this->backupFilePath);
common::unsetenv(kFilePath);

#ifdef _WIN32
this->filesystemRoot = "C:\\";
Expand All @@ -61,8 +57,8 @@ class SystemPathsFixture : public ::testing::Test
// Documentation inherited
public: virtual void TearDown()
{
putenv(const_cast<char*>(this->backupPluginPath.c_str()));
putenv(const_cast<char*>(this->backupFilePath.c_str()));
common::setenv(kPluginPath, this->backupPluginPath);
common::setenv(kFilePath, this->backupFilePath);
}

/// \brief Backup of plugin paths to be restored after the test
Expand All @@ -75,21 +71,18 @@ class SystemPathsFixture : public ::testing::Test
public: std::string filesystemRoot;
};

std::string SystemPathsJoin(const std::vector<std::string> &_paths)
{
return common::Join(_paths, common::SystemPaths::Delimiter());
}

/////////////////////////////////////////////////
TEST_F(SystemPathsFixture, SystemPaths)
{
common::SystemPaths paths;
std::string env_str("IGN_PLUGIN_PATH=/tmp/plugin");
env_str += ignition::common::SystemPaths::Delimiter();
env_str += "/test/plugin/now";

// The char* passed to putenv will continue to be stored after the lifetime
// of this function, so we should not pass it a pointer which has an automatic
// lifetime.
static char env[1024];
snprintf(env, sizeof(env), "%s", env_str.c_str());
putenv(env);
common::setenv(kPluginPath,
SystemPathsJoin({"/tmp/plugin", "/test/plugin/now"}));

common::SystemPaths paths;
const std::list<std::string> pathList3 = paths.PluginPaths();
EXPECT_EQ(static_cast<unsigned int>(2), pathList3.size());
EXPECT_STREQ("/tmp/plugin/", pathList3.front().c_str());
Expand All @@ -103,19 +96,12 @@ TEST_F(SystemPathsFixture, SystemPaths)
/////////////////////////////////////////////////
TEST_F(SystemPathsFixture, FileSystemPaths)
{
std::string env_str("IGN_FILE_PATH=/tmp/file");
env_str += ignition::common::SystemPaths::Delimiter();
env_str += "/test/file/now";

// The char* passed to putenv will continue to be stored after the lifetime
// of this function, so we should not pass it a pointer which has an automatic
// lifetime.
static char env[1024];
snprintf(env, sizeof(env), "%s", env_str.c_str());
putenv(env);
std::vector<std::string> filePaths {"/tmp/file",
"/test/file/now"};
common::setenv(kFilePath, SystemPathsJoin(filePaths));

common::SystemPaths paths;
EXPECT_EQ("IGN_FILE_PATH", paths.FilePathEnv());
EXPECT_EQ(kFilePath, paths.FilePathEnv());
const std::list<std::string> pathList3 = paths.FilePaths();
EXPECT_EQ(static_cast<unsigned int>(2), pathList3.size());
EXPECT_STREQ("/tmp/file/", pathList3.front().c_str());
Expand All @@ -136,12 +122,10 @@ TEST_F(SystemPathsFixture, FileSystemPaths)

EXPECT_EQ("", paths.FindFile("no_such_file"));
EXPECT_EQ("", paths.FindFile("test_f1"));
env_str += ignition::common::SystemPaths::Delimiter();
env_str += "test_dir1";

snprintf(env, sizeof(env), "%s", env_str.c_str());
putenv(env);
paths.SetFilePathEnv("IGN_FILE_PATH");
filePaths.push_back("test_dir1");
common::setenv(kFilePath, SystemPathsJoin(filePaths));
paths.SetFilePathEnv(kFilePath);
EXPECT_EQ(file1, paths.FindFile("test_f1")) << paths.FindFile("test_f1");
EXPECT_EQ(file1, paths.FindFile("model://test_f1"));
}
Expand Down Expand Up @@ -285,19 +269,14 @@ TEST_F(SystemPathsFixture, FindFileURI)
EXPECT_EQ(file2, sp.FindFileURI("osrf://test_f2"));

// URI + env var
static char env[1024];
snprintf(env, sizeof(env), "%s", ("IGN_FILE_PATH=" + dir1).c_str());
putenv(env);

sp.SetFilePathEnv("IGN_FILE_PATH");
EXPECT_EQ("IGN_FILE_PATH", sp.FilePathEnv());
common::setenv(kFilePath, dir1);
sp.SetFilePathEnv(kFilePath);
EXPECT_EQ(kFilePath, sp.FilePathEnv());
EXPECT_EQ(file1, sp.FindFileURI("anything://test_f1"));
EXPECT_NE(file2, sp.FindFileURI("anything://test_f2"));

std::string newEnv{"IGN_NEW_FILE_PATH"};
snprintf(env, sizeof(env), "%s", (newEnv + "=" + dir2).c_str());
putenv(env);

common::setenv(newEnv, dir2);
sp.SetFilePathEnv(newEnv);
EXPECT_EQ(newEnv, sp.FilePathEnv());
EXPECT_NE(file1, sp.FindFileURI("anything://test_f1"));
Expand Down Expand Up @@ -422,20 +401,10 @@ TEST_F(SystemPathsFixture, NormalizeDirectoryPath)
//////////////////////////////////////////////////
TEST_F(SystemPathsFixture, PathsFromEnv)
{
std::string env_str = "IGN_PLUGIN_PATH=/tmp/plugin";
env_str += ignition::common::SystemPaths::Delimiter();
env_str += "/test/plugin/now/";
env_str += ignition::common::SystemPaths::Delimiter();
env_str += "/tmp/plugin";

// The char* passed to putenv will continue to be stored after the lifetime
// of this function, so we should not pass it a pointer which has an automatic
// lifetime.
static char env[1024];
snprintf(env, sizeof(env), "%s", env_str.c_str());
putenv(env);

auto paths = ignition::common::SystemPaths::PathsFromEnv("IGN_PLUGIN_PATH");
common::setenv(kPluginPath,
SystemPathsJoin({"/tmp/plugin", "/test/plugin/now/", "/tmp/plugin"}));

auto paths = ignition::common::SystemPaths::PathsFromEnv(kPluginPath);

EXPECT_EQ(paths.size(), 2u);

Expand Down

0 comments on commit dfc3f10

Please sign in to comment.