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

Eric Fiselier via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue May 5 18:54:07 PDT 2020


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

When I initially wrote the filesystem tests, I had divided them between "static" and "dynamic"; that is, tests which modified the target filesystem and those which did not.

I was concerned that some targets supported by libc++ wouldn't have the runtime dependencies for "dynamic" tests available. I didn't want these targets to have no filesystem tests,
I tried to write a minimal subset of tests which were "static".

I don't know that my initial concerned panned out, so I have reserved support for removing it if needed. That said, I don't understand how this patch makes these tests pass on Windows.
If Windows doesn't support symlinks, how does dynamically creating them make a difference?

Because this change unknowingly broke tests, I'm going to revert it.
We can recommit it after ensuring there are no other accidental changes.



================
Comment at: libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.is_directory/is_directory.pass.cpp:66
 {
     const path p = StaticEnv::DNE;
     TEST_CHECK(is_directory(p) == false);
----------------
This test is no longer correct.
I assume this change broke or changed the meaning of other tests as well.

If we're creating the static test env using a RAII guard,
that RAII guard should be the only way to name the members of the static environment
and the old `StaticEnv::Foo` variables should be removed.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78200





More information about the libcxx-commits mailing list