[libcxx] [test] Use GetWindowsInaccessibleDir() instead of dirs with perms::none in fs.op.is_*

[curdeius]

LGTM.

[mstorsjo]

Ping @Mordante (who reviewed a similar patch earlier) for second approval

[mstorsjo]

Amended to remove xfails and to allow the dir to be missing. Has 1 approval from earlier.

[• Quuxplusone]

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

83

The general approach looks great to me, but I do think it'd make more sense to pull scoped_test_env env; up to the top of main regardless of _WIN32. Even if that "scope guard" is redundant on Windows, it feels sketchy to declare a "scope guard" variable inside an ifdef when its lifetime extends outside the ifdef.
(If you do this, obviously, do it consistently everywhere.)

[mstorsjo]

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

83

Hmm, I can see that it stylistically is weird in one way to have it outlive the ifdef, but I still think I prefer it this way - the ifdef branches set up different variables that then exist for the duration of the surrounding scope... If there's more opinions in this direction I can change it of course.

[curdeius]

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

83

I agree with Arthur, it is less surprising to have the guard in all branches of #if. Even if it's not necessary on Windows now, it may become necessary one day, this can avoid stupid mistakes then.

[mstorsjo]

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

83

Ok, thanks for chiming in. Updating then.

[mstorsjo]

Moved the scoped_test_env outside of the ifdef.

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

 {
     static_test_env static_env;
     const path p = static_env.DNE;
     TEST_CHECK(is_block_file(p) == false);
 }
 
 TEST_CASE(test_is_block_file_fails)
 {
+#ifdef _WIN32
+    // Windows doesn't support setting perms::none to trigger failures
+    // reading directories; test using a special inaccessible directory
+    // instead.
+    const path p = GetWindowsInaccessibleDir();
+    TEST_REQUIRE(!p.empty());
+#else
     scoped_test_env env;
     const path dir = env.create_dir("dir");
-    const path file = env.create_file("dir/file", 42);
+    const path p = env.create_file("dir/file", 42);
     permissions(dir, perms::none);
+#endif
 
     std::error_code ec;
-    TEST_CHECK(is_block_file(file, ec) == false);
+    TEST_CHECK(is_block_file(p, ec) == false);
     TEST_CHECK(ec);
 
-    TEST_CHECK_THROW(filesystem_error, is_block_file(file));
+    TEST_CHECK_THROW(filesystem_error, is_block_file(p));
 }
 
 TEST_SUITE_END()

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

 {
     static_test_env static_env;
     const path p = static_env.DNE;
     TEST_CHECK(is_character_file(p) == false);
 }
 
 TEST_CASE(test_is_character_file_fails)
 {
+#ifdef _WIN32
+    // Windows doesn't support setting perms::none to trigger failures
+    // reading directories; test using a special inaccessible directory
+    // instead.
+    const path p = GetWindowsInaccessibleDir();
+    TEST_REQUIRE(!p.empty());
+#else
     scoped_test_env env;
     const path dir = env.create_dir("dir");
-    const path file = env.create_file("dir/file", 42);
+    const path p = env.create_file("dir/file", 42);
     permissions(dir, perms::none);
+#endif
 
     std::error_code ec;
-    TEST_CHECK(is_character_file(file, ec) == false);
+    TEST_CHECK(is_character_file(p, ec) == false);
     TEST_CHECK(ec);
 
-    TEST_CHECK_THROW(filesystem_error, is_character_file(file));
+    TEST_CHECK_THROW(filesystem_error, is_character_file(p));
 }
 
 TEST_SUITE_END()

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

TEST_CASE(static_env_test)
     static_test_env static_env;
     TEST_CHECK(is_directory(static_env.Dir));
     TEST_CHECK(is_directory(static_env.SymlinkToDir));
     TEST_CHECK(!is_directory(static_env.File));
 }
 
 TEST_CASE(test_is_directory_fails)
 {
+#ifdef _WIN32
+    // Windows doesn't support setting perms::none to trigger failures
+    // reading directories; test using a special inaccessible directory
+    // instead.
+    const path p = GetWindowsInaccessibleDir();
+    TEST_REQUIRE(!p.empty());
+#else
     scoped_test_env env;
     const path dir = env.create_dir("dir");
-    const path dir2 = env.create_dir("dir/dir2");
+    const path p = env.create_dir("dir/dir2");
     permissions(dir, perms::none);
+#endif
 
     std::error_code ec;
-    TEST_CHECK(is_directory(dir2, ec) == false);
+    TEST_CHECK(is_directory(p, ec) == false);
     TEST_CHECK(ec);
 
-    TEST_CHECK_THROW(filesystem_error, is_directory(dir2));
+    TEST_CHECK_THROW(filesystem_error, is_directory(p));
 }
 
 TEST_SUITE_END()

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

 {
     static_test_env static_env;
     TEST_CHECK(is_empty(static_env.EmptyFile));
     TEST_CHECK(!is_empty(static_env.NonEmptyFile));
 }
 
 TEST_CASE(test_is_empty_fails)
 {
+#ifdef _WIN32
+    // Windows doesn't support setting perms::none to trigger failures
+    // reading directories; test using a special inaccessible directory
+    // instead.
+    const path p = GetWindowsInaccessibleDir();
+    TEST_REQUIRE(!p.empty());
+#else
     scoped_test_env env;
     const path dir = env.create_dir("dir");
-    const path dir2 = env.create_dir("dir/dir2");
+    const path p = env.create_dir("dir/dir2");
     permissions(dir, perms::none);
+#endif
 
     std::error_code ec;
-    TEST_CHECK(is_empty(dir2, ec) == false);
+    TEST_CHECK(is_empty(p, ec) == false);
     TEST_CHECK(ec);
 
-    TEST_CHECK_THROW(filesystem_error, is_empty(dir2));
+    TEST_CHECK_THROW(filesystem_error, is_empty(p));
 }
 
 TEST_CASE(test_directory_access_denied)
 {
+#ifdef _WIN32
+    // Windows doesn't support setting perms::none to trigger failures
+    // reading directories; test using a special inaccessible directory
+    // instead.
+    const path dir = GetWindowsInaccessibleDir();
+    TEST_REQUIRE(!dir.empty());
+#else
     scoped_test_env env;
     const path dir = env.create_dir("dir");
     const path file1 = env.create_file("dir/file", 42);
     permissions(dir, perms::none);
+#endif
 
     std::error_code ec = GetTestEC();
     TEST_CHECK(is_empty(dir, ec) == false);
     TEST_CHECK(ec);
     TEST_CHECK(ec != GetTestEC());
 
     TEST_CHECK_THROW(filesystem_error, is_empty(dir));
 }

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

 {
     static_test_env static_env;
     const path p = static_env.DNE;
     TEST_CHECK(is_fifo(p) == false);
 }
 
 TEST_CASE(test_is_fifo_fails)
 {
+#ifdef _WIN32
+    // Windows doesn't support setting perms::none to trigger failures
+    // reading directories; test using a special inaccessible directory
+    // instead.
+    const path p = GetWindowsInaccessibleDir();
+    TEST_REQUIRE(!p.empty());
+#else
     scoped_test_env env;
     const path dir = env.create_dir("dir");
-    const path file = env.create_file("dir/file", 42);
+    const path p = env.create_file("dir/file", 42);
     permissions(dir, perms::none);
+#endif
 
     std::error_code ec;
-    TEST_CHECK(is_fifo(file, ec) == false);
+    TEST_CHECK(is_fifo(p, ec) == false);
     TEST_CHECK(ec);
 
-    TEST_CHECK_THROW(filesystem_error, is_fifo(file));
+    TEST_CHECK_THROW(filesystem_error, is_fifo(p));
 }
 
 TEST_SUITE_END()

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

 {
     static_test_env static_env;
     const path p = static_env.DNE;
     TEST_CHECK(is_other(p) == false);
 }
 
 TEST_CASE(test_is_other_fails)
 {
+#ifdef _WIN32
+    // Windows doesn't support setting perms::none to trigger failures
+    // reading directories; test using a special inaccessible directory
+    // instead.
+    const path p = GetWindowsInaccessibleDir();
+    TEST_REQUIRE(!p.empty());
+#else
     scoped_test_env env;
     const path dir = env.create_dir("dir");
-    const path file = env.create_file("dir/file", 42);
+    const path p = env.create_file("dir/file", 42);
     permissions(dir, perms::none);
+#endif
 
     std::error_code ec;
-    TEST_CHECK(is_other(file, ec) == false);
+    TEST_CHECK(is_other(p, ec) == false);
     TEST_CHECK(ec);
 
-    TEST_CHECK_THROW(filesystem_error, is_other(file));
+    TEST_CHECK_THROW(filesystem_error, is_other(p));
 }
 
 TEST_SUITE_END()

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

TEST_CASE(test_exist_not_found)
     TEST_CHECK(is_regular_file(p) == false);
     std::error_code ec;
     TEST_CHECK(is_regular_file(p, ec) == false);
     TEST_CHECK(ec);
 }
 
 TEST_CASE(test_is_regular_file_fails)
 {
+#ifdef _WIN32
+    // Windows doesn't support setting perms::none to trigger failures
+    // reading directories; test using a special inaccessible directory
+    // instead.
+    const path p = GetWindowsInaccessibleDir();
+    TEST_REQUIRE(!p.empty());
+#else
     scoped_test_env env;
     const path dir = env.create_dir("dir");
-    const path file = env.create_file("dir/file", 42);
+    const path p = env.create_file("dir/file", 42);
     permissions(dir, perms::none);
+#endif
 
     std::error_code ec;
-    TEST_CHECK(is_regular_file(file, ec) == false);
+    TEST_CHECK(is_regular_file(p, ec) == false);
     TEST_CHECK(ec);
 
-    TEST_CHECK_THROW(filesystem_error, is_regular_file(file));
+    TEST_CHECK_THROW(filesystem_error, is_regular_file(p));
 }
 
 TEST_SUITE_END()

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

 {
     static_test_env static_env;
     const path p = static_env.DNE;
     TEST_CHECK(is_socket(p) == false);
 }
 
 TEST_CASE(test_is_socket_fails)
 {
+#ifdef _WIN32
+    // Windows doesn't support setting perms::none to trigger failures
+    // reading directories; test using a special inaccessible directory
+    // instead.
+    const path p = GetWindowsInaccessibleDir();
+    TEST_REQUIRE(!p.empty());
+#else
     scoped_test_env env;
     const path dir = env.create_dir("dir");
-    const path file = env.create_file("dir/file", 42);
+    const path p = env.create_file("dir/file", 42);
     permissions(dir, perms::none);
+#endif
 
     std::error_code ec;
-    TEST_CHECK(is_socket(file, ec) == false);
+    TEST_CHECK(is_socket(p, ec) == false);
     TEST_CHECK(ec);
 
-    TEST_CHECK_THROW(filesystem_error, is_socket(file));
+    TEST_CHECK_THROW(filesystem_error, is_socket(p));
 }
 
 TEST_SUITE_END()

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

TEST_CASE(test_exist_not_found)
     TEST_CHECK(is_symlink(p) == false);
     std::error_code ec;
     TEST_CHECK(is_symlink(p, ec) == false);
     TEST_CHECK(ec);
 }
 
 TEST_CASE(test_is_symlink_fails)
 {
+#ifdef _WIN32
+    // Windows doesn't support setting perms::none to trigger failures
+    // reading directories; test using a special inaccessible directory
+    // instead.
+    const path p = GetWindowsInaccessibleDir();
+    TEST_REQUIRE(!p.empty());
+#else
     scoped_test_env env;
     const path dir = env.create_dir("dir");
-    const path file = env.create_file("dir/file", 42);
+    const path p = env.create_file("dir/file", 42);
     permissions(dir, perms::none);
+#endif
 
     std::error_code ec;
-    TEST_CHECK(is_symlink(file, ec) == false);
+    TEST_CHECK(is_symlink(p, ec) == false);
     TEST_CHECK(ec);
 
-    TEST_CHECK_THROW(filesystem_error, is_symlink(file));
+    TEST_CHECK_THROW(filesystem_error, is_symlink(p));
 }
 
 TEST_SUITE_END()