This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] [test] Split the file_time_type synopsis test
ClosedPublic

Authored by mstorsjo on Oct 16 2020, 3:38 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rGb4a289b03ced: [libcxx] [test] Split the file_time_type synopsis test
Summary

Split the resolution check to a separate test, which is marked as unsupported on windows.

On windows (both with MS STL and libstdc++), the file time has 100 ns resolution; the standard doesn't mandate a specific resolution.

Diff Detail

Event Timeline

mstorsjo created this revision.Oct 16 2020, 3:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 16 2020, 3:38 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
mstorsjo requested review of this revision.Oct 16 2020, 3:38 AM
ldionne requested changes to this revision.Oct 16 2020, 5:43 AM
ldionne added a subscriber: ldionne.
ldionne added inline comments.
libcxx/test/std/input.output/filesystems/fs.filesystem.synopsis/file_time_type.pass.cpp
35

I would rather you mark the whole test as // UNSUPPORTED on Windows. Or split this part of the test into another test, and mark that as unsupported.

I'm trying to get rid of platform specific #ifs in the test suite when possible. It gives a better overview of what is and isn't supported when running the test suite on various platforms.

This revision now requires changes to proceed.Oct 16 2020, 5:43 AM
mstorsjo updated this revision to Diff 298718.Oct 16 2020, 12:32 PM
mstorsjo retitled this revision from [libcxx] [test] Ifdef out the time point resolution and range check on windows to [libcxx] [test] Split the file_time_type synopsis test.
mstorsjo edited the summary of this revision. (Show Details)

Split the test into two, instead of adding ifdefs.

ldionne accepted this revision.Oct 20 2020, 2:04 PM

LGTM with the suggested change.

libcxx/test/std/input.output/filesystems/fs.filesystem.synopsis/file_time_type_resolution.pass.cpp
24 ↗(On Diff #298718)

Can you rename to file_time_type_resolution.compile.pass.cpp and just use:

using namespace fs;
using Dur = file_time_type::duration;
using Period = Dur::period;
ASSERT_SAME_TYPE(Period, std::nano);

No need for main or test_time_point_resolution_and_range().

This revision is now accepted and ready to land.Oct 20 2020, 2:04 PM
This revision was automatically updated to reflect the committed changes.