This is an archive of the discontinued LLVM Phabricator instance.

Differential D98138

[libcxx] [test] Don't test CharFile (/dev/null) on windows
ClosedPublic

Authored by mstorsjo on Mar 7 2021, 1:57 AM.

Details

Reviewers
• Quuxplusone
curdeius
Group Reviewers
Restricted Project
Commits
rG3a11a41795be: [libcxx] [test] Don't test CharFile (/dev/null) on windows

Diff Detail

Unit TestsFailed

TimeTest
1,340 mslibcxx CI Apple system -fno-exceptions > libc++.std/numerics/numarray/template_valarray/valarray_sub::gslice_non_const.pass.cpp
1,300 mslibcxx CI Apple system -fno-exceptions > libc++.std/numerics/numarray/template_valarray/valarray_sub::indirect_array_const.pass.cpp
1,310 mslibcxx CI Apple system -fno-exceptions > libc++.std/numerics/numarray/template_valarray/valarray_sub::indirect_array_non_const.pass.cpp
1,030 mslibcxx CI Apple system -fno-exceptions > libc++.std/numerics/numarray/template_valarray/valarray_sub::slice_const.pass.cpp
1,070 mslibcxx CI Apple system -fno-exceptions > libc++.std/numerics/numarray/template_valarray/valarray_sub::slice_non_const.pass.cpp
View Full Test Results (2,014 Failed)

Event Timeline

mstorsjo requested review of this revision.Mar 7 2021, 1:57 AM
mstorsjo created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 7 2021, 1:57 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
Harbormaster completed remote builds in B92545: Diff 328852.Mar 7 2021, 4:03 AM
mstorsjo updated this revision to Diff 328872.Mar 7 2021, 8:40 AM

Reupload to retry CI

• Quuxplusone accepted this revision as: • Quuxplusone.Mar 7 2021, 9:06 AM
• Quuxplusone added a subscriber: • Quuxplusone.
• Quuxplusone added inline comments.
libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.status/status.pass.cpp
116–117

You might adjust this comment to // file_type::block is tested elsewhere, just so that people don't assume it's related to the #ifndef on the next line.
(And ditto in the other test file, as well.)

curdeius added a subscriber: curdeius.Mar 7 2021, 9:35 AM

Have you heard of nul special file on Windows?
https://gcc.gnu.org/legacy-ml/gcc-patches/2005-05/msg01793.html
I haven't tested it yet but maybe using it on Win instead of /dev/null might be a good idea?

Harbormaster completed remote builds in B92561: Diff 328872.Mar 7 2021, 10:18 AM
mstorsjo added a comment.Mar 7 2021, 10:32 AM
In D98138#2609952, @curdeius wrote:

Have you heard of nul special file on Windows?
https://gcc.gnu.org/legacy-ml/gcc-patches/2005-05/msg01793.html
I haven't tested it yet but maybe using it on Win instead of /dev/null might be a good idea?

Good point, but neither our implementation nor the MS STL one actually identify files as file_type::character, see https://github.com/microsoft/STL/blob/main/stl/inc/filesystem#L1882 which says:

character, // not used in this implementation; theoretically some special files like CON might qualify, but querying for this is extremely expensive and unlikely to be useful in practice
libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.status/status.pass.cpp
116–117

Sure, I can do that.

curdeius accepted this revision.Mar 7 2021, 11:39 AM

LGTM. Could you put a similar comment (but concerning only Windows of course) near file_type::character declaration as well?

This revision is now accepted and ready to land.Mar 7 2021, 11:39 AM
mstorsjo added a comment.Mar 7 2021, 1:25 PM
In D98138#2610071, @curdeius wrote:

LGTM. Could you put a similar comment (but concerning only Windows of course) near file_type::character declaration as well?

In the header? Sure, I can make a separate patch for that.

Closed by commit rG3a11a41795be: [libcxx] [test] Don't test CharFile (/dev/null) on windows (authored by mstorsjo). · Explain WhyMar 7 2021, 1:32 PM
This revision was automatically updated to reflect the committed changes.
mstorsjo added a commit: rG3a11a41795be: [libcxx] [test] Don't test CharFile (/dev/null) on windows.
mstorsjo mentioned this in D98155: [libcxx] Document windows specifics regarding file_type and perms. NFC..Mar 7 2021, 1:51 PM
mstorsjo mentioned this in rGb9f169fb7dcd: [libcxx] Document windows specifics regarding file_type and perms. NFC..Mar 8 2021, 5:27 AM

Revision Contents

PathSize
libcxx/
test/
std/
input.output/
filesystems/
fs.op.funcs/
fs.op.status/
2 lines
fs.op.symlink_status/
2 lines

Diff 328852

libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.status/status.pass.cpp

Show First 20 Lines • Show All 107 Lines • ▼ Show 20 LinesTEST_CASE(status_file_types_test)
struct TestCase { struct TestCase {
path p; path p;
file_type expect_type; file_type expect_type;
} cases[] = { } cases[] = {
{static_env.File, file_type::regular}, {static_env.File, file_type::regular},
{static_env.SymlinkToFile, file_type::regular}, {static_env.SymlinkToFile, file_type::regular},
{static_env.Dir, file_type::directory}, {static_env.Dir, file_type::directory},
{static_env.SymlinkToDir, file_type::directory}, {static_env.SymlinkToDir, file_type::directory},
// Block files tested elsewhere // Block files tested elsewhere
#ifndef _WIN32
QuuxplusoneUnsubmitted
Not Done
ReplyInline Actions

You might adjust this comment to // file_type::block is tested elsewhere, just so that people don't assume it's related to the #ifndef on the next line.
(And ditto in the other test file, as well.)

Quuxplusone: You might adjust this comment to `// file_type::block is tested elsewhere`, just so that people…
mstorsjoAuthorUnsubmitted
Done
ReplyInline Actions

Sure, I can do that.

mstorsjo: Sure, I can do that.
{static_env.CharFile, file_type::character}, {static_env.CharFile, file_type::character},
#endif
#if !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(_WIN32) // No support for domain sockets #if !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(_WIN32) // No support for domain sockets
{env.create_socket("socket"), file_type::socket}, {env.create_socket("socket"), file_type::socket},
#endif #endif
#ifndef _WIN32 #ifndef _WIN32
{env.create_fifo("fifo"), file_type::fifo} {env.create_fifo("fifo"), file_type::fifo}
#endif #endif
}; };
for (const auto& TC : cases) { for (const auto& TC : cases) {
▲ Show 20 Lines • Show All 44 Lines • Show Last 20 Lines

libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.symlink_status/symlink_status.pass.cpp

Show First 20 Lines • Show All 117 Lines • ▼ Show 20 Lines struct TestCase {
file_type expect_type; file_type expect_type;
} cases[] = { } cases[] = {
{static_env.BadSymlink, file_type::symlink}, {static_env.BadSymlink, file_type::symlink},
{static_env.File, file_type::regular}, {static_env.File, file_type::regular},
{static_env.SymlinkToFile, file_type::symlink}, {static_env.SymlinkToFile, file_type::symlink},
{static_env.Dir, file_type::directory}, {static_env.Dir, file_type::directory},
{static_env.SymlinkToDir, file_type::symlink}, {static_env.SymlinkToDir, file_type::symlink},
// Block files tested elsewhere // Block files tested elsewhere
#ifndef _WIN32
{static_env.CharFile, file_type::character}, {static_env.CharFile, file_type::character},
#endif
#if !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(_WIN32) // No support for domain sockets #if !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(_WIN32) // No support for domain sockets
{env.create_socket("socket"), file_type::socket}, {env.create_socket("socket"), file_type::socket},
#endif #endif
#ifndef _WIN32 #ifndef _WIN32
{env.create_fifo("fifo"), file_type::fifo} {env.create_fifo("fifo"), file_type::fifo}
#endif #endif
}; };
for (const auto& TC : cases) { for (const auto& TC : cases) {
▲ Show 20 Lines • Show All 62 Lines • Show Last 20 Lines