[libcxx-commits] [PATCH] D78200: [libc++] [test] Generate static_test_env on the fly

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon May 4 10:09:44 PDT 2020


ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

In D78200#2017911 <https://reviews.llvm.org/D78200#2017911>, @broadwaylamb wrote:

> @ldionne @EricWF am I getting it right that this patch is the wrong direction? How the described Windows issue could be resolved otherwise?


I personally think it's the right direction. I believe Eric's point is that creating the static environment *statically* has the benefit that we know exactly what input our tests are running on, whereas we don't when we create the environment on the fly (e.g. there could be a bug in how we generate the env). However, given the difficulty with Windows, I'm OK with that.

Unless we can find another way to solve this problem, I believe we should move forward with this patch so as to enable Filesystem testing from a Windows host.



================
Comment at: libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.current_path/current_path.pass.cpp:54
 {
-    const path new_path = StaticEnv::Dir;
+    CWDGuard guard;
+    static_test_env static_env;
----------------
Just to confirm, you add the guard here because we change the current directory, but the directory is then removed so the test would fail if we didn't go back to the previous cwd at the end?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78200/new/

https://reviews.llvm.org/D78200





More information about the libcxx-commits mailing list