[libcxx-commits] [libcxx] 05bc588 - [libc++] Do not rely on the environment to run filesystem tests
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Mar 31 06:05:09 PDT 2020
Author: Louis Dionne
Date: 2020-03-31T09:03:17-04:00
New Revision: 05bc588abbc38ad1f8f67994ec3bb606f0aaa983
URL: https://github.com/llvm/llvm-project/commit/05bc588abbc38ad1f8f67994ec3bb606f0aaa983
DIFF: https://github.com/llvm/llvm-project/commit/05bc588abbc38ad1f8f67994ec3bb606f0aaa983.diff
LOG: [libc++] Do not rely on the environment to run filesystem tests
Previously, filesystem tests would require LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT
to be present in the environment and to match the value provided when
compiling, as a macro. This has the problem that it only allows for the
filesystem tests to be run on the same machine they are created.
Instead, we create a temporary directory for each test. Technically,
this is tricky to do because we're relying on some of the code that
we're testing to do this. However, there's no other portable way of
creating temporary direcories in C++, so this is difficult to avoid.
Differential Revision: https://reviews.llvm.org/D76731
Added:
Modified:
libcxx/test/CMakeLists.txt
libcxx/test/support/filesystem_dynamic_test_helper.py
libcxx/test/support/filesystem_test_helper.h
libcxx/utils/libcxx/test/config.py
Removed:
################################################################################
diff --git a/libcxx/test/CMakeLists.txt b/libcxx/test/CMakeLists.txt
index e63e18ac1319..78a7b88c5db1 100644
--- a/libcxx/test/CMakeLists.txt
+++ b/libcxx/test/CMakeLists.txt
@@ -142,9 +142,6 @@ if (LIBCXX_CONFIGURE_IDE)
set(STATIC_ROOT ${LIBCXX_SOURCE_DIR}/test/std/input.output/filesystems/Inputs/static_test_env)
add_definitions(-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="${STATIC_ROOT}")
- set(DYNAMIC_ROOT ${LIBCXX_BINARY_DIR}/test/filesystem/Output/dynamic_env)
- add_definitions(-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="${DYNAMIC_ROOT}")
-
set(DYNAMIC_HELPER "python ${LIBCXX_SOURCE_DIR}/test/support/filesystem_dynamic_test_helper.py ")
add_definitions(-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="${DYNAMIC_HELPER}")
diff --git a/libcxx/test/support/filesystem_dynamic_test_helper.py b/libcxx/test/support/filesystem_dynamic_test_helper.py
index 078958274978..c769ac8e079b 100644
--- a/libcxx/test/support/filesystem_dynamic_test_helper.py
+++ b/libcxx/test/support/filesystem_dynamic_test_helper.py
@@ -8,20 +8,14 @@
or sys.platform.startswith('cygwin') or sys.platform.startswith('freebsd') \
or sys.platform.startswith('netbsd')
-def env_path():
- ep = os.environ.get('LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT')
- assert ep is not None
- ep = os.path.realpath(ep)
- assert os.path.isdir(ep)
- return ep
+test_root = None
-env_path_global = env_path()
-
-# Make sure we don't try and write outside of env_path.
+# Make sure we don't try and write outside of the test root.
# All paths used should be sanitized
def sanitize(p):
+ assert os.path.isdir(test_root)
p = os.path.realpath(p)
- if os.path.commonprefix([env_path_global, p]):
+ if os.path.commonprefix([test_root, p]):
return p
assert False
@@ -44,7 +38,7 @@ def clean_recursive(root_p):
def init_test_directory(root_p):
- root_p = sanitize(root_p)
+ root_p = os.path.realpath(root_p)
assert not os.path.exists(root_p)
os.makedirs(root_p)
@@ -87,6 +81,7 @@ def create_socket(source):
if __name__ == '__main__':
- command = " ".join(sys.argv[1:])
+ test_root = os.path.realpath(sys.argv[1])
+ command = " ".join(sys.argv[2:])
eval(command)
sys.exit(0)
diff --git a/libcxx/test/support/filesystem_test_helper.h b/libcxx/test/support/filesystem_test_helper.h
index 8161820d05da..2c15606cbec2 100644
--- a/libcxx/test/support/filesystem_test_helper.h
+++ b/libcxx/test/support/filesystem_test_helper.h
@@ -103,9 +103,6 @@ static const fs::path RecDirFollowSymlinksIterationList[] = {
#endif // LIBCXX_FILESYSTEM_STATIC_TEST_ROOT
-#ifndef LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT
-#warning LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT must be defined
-#else // LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT
#ifndef LIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER
#error LIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER must be defined
@@ -127,8 +124,16 @@ inline char random_hex_char() {
struct scoped_test_env
{
- scoped_test_env() : test_root(random_env_path())
- { fs_helper_run(fs_make_cmd("init_test_directory", test_root)); }
+ scoped_test_env() : test_root(random_path())
+ {
+ fs_helper_run(fs_make_cmd("init_test_directory", test_root));
+
+ // Ensure that the root_path is fully resolved, i.e. it contains no
+ // symlinks. The filesystem tests depend on that. We do this after
+ // creating the root_path, because `fs::canonical` requires the
+ // path to exist.
+ test_root = fs::canonical(test_root);
+ }
~scoped_test_env()
{ fs_helper_run(fs_make_cmd("destroy_test_directory", test_root)); }
@@ -229,7 +234,7 @@ struct scoped_test_env
}
#endif
- fs::path const test_root;
+ fs::path test_root;
private:
static std::string unique_path_suffix() {
@@ -243,10 +248,10 @@ struct scoped_test_env
// This could potentially introduce a filesystem race with other tests
// running at the same time, but oh well, it's just test code.
- static inline fs::path random_env_path() {
- static const char* env_path = LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT;
- fs::path p = fs::path(env_path) / unique_path_suffix();
- assert(p.parent_path() == env_path);
+ static inline fs::path random_path() {
+ fs::path tmp = fs::temp_directory_path();
+ fs::path p = fs::path(tmp) / unique_path_suffix();
+ assert(p.parent_path() == tmp);
return p;
}
@@ -270,39 +275,15 @@ struct scoped_test_env
return cmd_name + "(" + make_arg(arg1) + ", " + make_arg(arg2) + ")";
}
- static inline void fs_helper_run(std::string const& raw_cmd) {
- // check that the fs test root in the environment matches what we were
- // compiled with.
- static bool checked = checkDynamicTestRoot();
- ((void)checked);
+ void fs_helper_run(std::string const& raw_cmd) {
std::string cmd = LIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER;
+ cmd += " \"" + test_root.native() + "\"";
cmd += " \"" + raw_cmd + "\"";
int ret = std::system(cmd.c_str());
assert(ret == 0);
}
-
- static bool checkDynamicTestRoot() {
- // LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT is expected not to contain symlinks.
- char* fs_root = std::getenv("LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT");
- if (!fs_root) {
- std::printf("ERROR: LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT must be a defined "
- "environment variable when running the test.\n");
- std::abort();
- }
- if (std::string(fs_root) != LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT) {
- std::printf("ERROR: LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT environment variable"
- " must have the same value as when the test was compiled.\n");
- std::printf(" Current Value: '%s'\n", fs_root);
- std::printf(" Expected Value: '%s'\n", LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT);
- std::abort();
- }
- return true;
- }
-
};
-#endif // LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT
-
// Misc test types
#define MKSTR(Str) {Str, TEST_CONCAT(L, Str), TEST_CONCAT(u, Str), TEST_CONCAT(U, Str)}
diff --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index d53bd6cbeb36..4babf50f946b 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -731,14 +731,6 @@ def configure_filesystem_compile_flags(self):
assert os.path.isdir(static_env)
self.cxx.compile_flags += ['-DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="%s"' % static_env]
- dynamic_env = os.path.join(self.config.test_exec_root,
- 'filesystem', 'Output', 'dynamic_env')
- dynamic_env = os.path.realpath(dynamic_env)
- if not os.path.isdir(dynamic_env):
- os.makedirs(dynamic_env)
- self.cxx.compile_flags += ['-DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="%s"' % dynamic_env]
- self.exec_env['LIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT'] = ("%s" % dynamic_env)
-
dynamic_helper = os.path.join(self.libcxx_src_root, 'test', 'support',
'filesystem_dynamic_test_helper.py')
assert os.path.isfile(dynamic_helper)
More information about the libcxx-commits
mailing list