Page MenuHomePhabricator

[libc++] Do not rely on the environment to run filesystem tests
ClosedPublic

Authored by ldionne on Mar 24 2020, 2:11 PM.

Details

Summary

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.

Diff Detail

Event Timeline

ldionne created this revision.Mar 24 2020, 2:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2020, 2:11 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

This is awesome. I'm working on running filesystem tests on a remote machine right now, this should be of much help.

EricWF added a subscriber: EricWF.Mar 26 2020, 8:02 AM
EricWF added inline comments.
libcxx/test/support/filesystem_test_helper.h
232

Why did you drop the const?

ldionne marked 2 inline comments as done.Mar 26 2020, 8:10 AM
ldionne added inline comments.
libcxx/test/support/filesystem_test_helper.h
232

Because of test_root = fs::canonical(test_root); above in the constructor. Note that I can't set that in the initializer list, because fs::canonical requires the test_root to exist, which is set up in the constructor body.

EricWF accepted this revision.Mar 31 2020, 12:42 AM
EricWF added inline comments.
libcxx/test/support/filesystem_test_helper.h
232

Alright. I'm OK with this.

I would rather it be const, but *shrug*.

This revision is now accepted and ready to land.Mar 31 2020, 12:42 AM
This revision was automatically updated to reflect the committed changes.