diff --git a/tests/std/include/temp_file_name.hpp b/tests/std/include/temp_file_name.hpp index 79991f56f2..45e3e171eb 100644 --- a/tests/std/include/temp_file_name.hpp +++ b/tests/std/include/temp_file_name.hpp @@ -7,11 +7,11 @@ #include [[nodiscard]] inline std::string temp_file_name() { - std::string ret{"temp_file_"}; + std::string ret{"msvc_stl_"}; std::uniform_int_distribution dist{0, 15}; std::random_device rd; - for (int i = 0; i < 64; ++i) { // 64 hexits = 256 bits of entropy + for (int i = 0; i < 32; ++i) { // 32 hexits = 128 bits of entropy ret.push_back("0123456789ABCDEF"[dist(rd)]); } diff --git a/tests/std/include/test_filesystem_support.hpp b/tests/std/include/test_filesystem_support.hpp index 3aa5986565..34c338b960 100644 --- a/tests/std/include/test_filesystem_support.hpp +++ b/tests/std/include/test_filesystem_support.hpp @@ -6,32 +6,31 @@ #define _SILENCE_EXPERIMENTAL_FILESYSTEM_DEPRECATION_WARNING #include -#include #include #include #include #include #include -inline std::string get_test_directory_subname(const char* const testName, const size_t testNameLength) { +template +std::string get_test_directory_subname(const T& testName) { using namespace std; random_device rd; uniform_int_distribution<> dist(0, 15); - string subName(testName, testNameLength); + string subName(testName); subName.push_back('_'); - generate_n(back_inserter(subName), 16, [&] { return "0123456789ABCDEF"[dist(rd)]; }); + generate_n(back_inserter(subName), 32, [&] { return "0123456789ABCDEF"[dist(rd)]; }); return subName; } -inline std::experimental::filesystem::path get_test_directory(const char* const testName) { - return std::experimental::filesystem::temp_directory_path() - / get_test_directory_subname(testName, strlen(testName)); +inline std::experimental::filesystem::path get_experimental_test_directory(const char* const testName) { + return std::experimental::filesystem::temp_directory_path() / get_test_directory_subname(testName); } #if _HAS_CXX17 #include -inline std::filesystem::path get_new_test_directory(std::string_view testName) { - return std::filesystem::temp_directory_path() / get_test_directory_subname(testName.data(), testName.size()); +inline std::filesystem::path get_test_directory(std::string_view testName) { + return std::filesystem::temp_directory_path() / get_test_directory_subname(testName); } #endif // _HAS_CXX17 diff --git a/tests/std/tests/Dev11_1066931_filesystem_rename_noop/test.cpp b/tests/std/tests/Dev11_1066931_filesystem_rename_noop/test.cpp index 27300609c4..dd727839db 100644 --- a/tests/std/tests/Dev11_1066931_filesystem_rename_noop/test.cpp +++ b/tests/std/tests/Dev11_1066931_filesystem_rename_noop/test.cpp @@ -345,7 +345,7 @@ int main() { error_code ec; const auto previousCd = fs::current_path(ec); assert_success(ec); - const auto testDir = get_test_directory("filesystem_rename_noop"); + const auto testDir = get_experimental_test_directory("filesystem_rename_noop"); printf("changing directory to \"%ls\"\n", testDir.native().c_str()); fs::create_directory(testDir, ec); assert_success(ec); diff --git a/tests/std/tests/P0218R1_filesystem/test.cpp b/tests/std/tests/P0218R1_filesystem/test.cpp index 305990b95a..c7907450e4 100644 --- a/tests/std/tests/P0218R1_filesystem/test.cpp +++ b/tests/std/tests/P0218R1_filesystem/test.cpp @@ -53,10 +53,10 @@ template return str.size() >= prefix.size() && Traits::compare(str.data(), prefix.data(), prefix.size()) == 0; } -struct test_temp_directory { - error_code ec; +struct [[nodiscard]] test_temp_directory { path directoryPath; - explicit test_temp_directory(const string_view testName) : directoryPath(get_new_test_directory(testName)) { + explicit test_temp_directory(const string_view testName) : directoryPath(get_test_directory(testName)) { + error_code ec; remove_all(directoryPath, ec); if (ec) { wcerr << L"Warning, couldn't clean up " << directoryPath << L" before test.\n"; @@ -68,7 +68,11 @@ struct test_temp_directory { } } + test_temp_directory(const test_temp_directory&) = delete; + test_temp_directory& operator=(const test_temp_directory&) = delete; + ~test_temp_directory() noexcept { + error_code ec; remove_all(directoryPath, ec); if (ec) { wcerr << L"Warning, couldn't clean up " << directoryPath << L" after test.\n"; @@ -1420,8 +1424,8 @@ void test_recursive_directory_iterator() { test_directory_iterator_common_parts("recursive_directory_iterator"sv); { - const test_temp_directory recursiveTests("recursive_directory_iterator specific"sv); - create_file_containing(recursiveTests.directoryPath / L"a.txt"sv, L"hello"); + const test_temp_directory tempDir("recursive_directory_iterator-specific"sv); + create_file_containing(tempDir.directoryPath / L"a.txt"sv, L"hello"); // _NODISCARD directory_options options() const; // _NODISCARD int depth() const; @@ -1430,7 +1434,7 @@ void test_recursive_directory_iterator() { // void disable_recursion_pending(); { error_code ec; - recursive_directory_iterator good_dir(recursiveTests.directoryPath, directory_options::none, ec); + recursive_directory_iterator good_dir(tempDir.directoryPath, directory_options::none, ec); if (!EXPECT(good(ec))) { return; } @@ -1438,11 +1442,11 @@ void test_recursive_directory_iterator() { EXPECT(good_dir.options() == directory_options::none); recursive_directory_iterator good_dir2( - recursiveTests.directoryPath, directory_options::skip_permission_denied, ec); + tempDir.directoryPath, directory_options::skip_permission_denied, ec); EXPECT(good_dir2.options() == directory_options::skip_permission_denied); recursive_directory_iterator good_dir3( - recursiveTests.directoryPath, directory_options::follow_directory_symlink, ec); + tempDir.directoryPath, directory_options::follow_directory_symlink, ec); EXPECT(good_dir3.options() == directory_options::follow_directory_symlink); EXPECT(good_dir.depth() == 0); @@ -1461,7 +1465,7 @@ void test_recursive_directory_iterator() { // void pop(); { - recursive_directory_iterator good_dir(recursiveTests.directoryPath, directory_options::none); + recursive_directory_iterator good_dir(tempDir.directoryPath, directory_options::none); good_dir.pop(); EXPECT(good_dir == recursive_directory_iterator{}); } @@ -1481,10 +1485,10 @@ void test_recursive_directory_iterator() { // Also test VSO-649431 follow_directory_symlinks with a broken symlink causes iteration to break { - const test_temp_directory followSymlinkTests("recursive_directory_iterator_VSO-649431"sv); - const path aaa = followSymlinkTests.directoryPath / L"aaa"sv; - const path bbb = followSymlinkTests.directoryPath / L"bbb"sv; - const path ccc = followSymlinkTests.directoryPath / L"ccc"sv; + const test_temp_directory tempDir("recursive_directory_iterator-VSO-649431"sv); + const path aaa = tempDir.directoryPath / L"aaa"sv; + const path bbb = tempDir.directoryPath / L"bbb"sv; + const path ccc = tempDir.directoryPath / L"ccc"sv; error_code ec; create_directory_symlink(nonexistentPaths[0], bbb, ec); if (ec) { @@ -1496,7 +1500,7 @@ void test_recursive_directory_iterator() { directory_options::follow_directory_symlink, directory_options::skip_permission_denied, directory_options::follow_directory_symlink | directory_options::skip_permission_denied}; for (const auto& option : options) { - recursive_directory_iterator first(followSymlinkTests.directoryPath, option); + recursive_directory_iterator first(tempDir.directoryPath, option); assert(first != recursive_directory_iterator{}); EXPECT(first->is_directory()); EXPECT(!first->is_symlink()); @@ -2883,7 +2887,7 @@ void test_invalid_conversions() { } void test_status() { - const test_temp_directory tempDir("test_status"sv); + const test_temp_directory tempDir("status"sv); const path& testDir = tempDir.directoryPath; const path testFile(testDir / L"test_file"sv); const path testLink(testDir / L"test_link"sv); @@ -3328,9 +3332,9 @@ void test_rename() { const path fileA(tempDir.directoryPath / L"filea.txt"sv); const path fileB(tempDir.directoryPath / L"fileb.txt"sv); - create_directories(dir.native(), ec); + create_directories(dir, ec); EXPECT(good(ec)); - create_directory(otherDir.native(), ec); + create_directory(otherDir, ec); EXPECT(good(ec)); create_file_containing(fileA, L"hello"); create_file_containing(fileB, L"world"); @@ -3342,15 +3346,26 @@ void test_rename() { EXPECT(good(ec)); EXPECT(read_file_contents(fileA) == L"hello"); +#ifndef _MSVC_INTERNAL_TESTING // TRANSITION, skip this for all MSVC-internal test runs. + // As of 2024-05-09, these rename() tests sporadically fail in MSVC-internal private test runs with + // "Access is denied" error codes. We've never observed such failures in MSVC-internal PR/CI checks, + // MSVC-internal local test runs, GitHub PR/CI checks, or GitHub local test runs. There's no significant + // compiler interaction here, so we can live with GitHub-only test coverage. Although we don't know the + // root cause, we suspect that this is related to the physical machines that are used for MSVC-internal + // private test runs, so we should check whether they've been replaced in a year or two. + // If new_p resolves to an existing non-directory file, new_p is removed rename(fileA, fileB, ec); EXPECT(good(ec)); - EXPECT(!exists(fileA.native())); + EXPECT(!exists(fileA)); EXPECT(read_file_contents(fileB) == L"hello"); // Standard rename where target doesn't exist - rename(fileB, fileA); + rename(fileB, fileA, ec); + EXPECT(good(ec)); + EXPECT(!exists(fileB)); EXPECT(read_file_contents(fileA) == L"hello"); +#endif // ^^^ no workaround ^^^ // Bad cases EXPECT(throws_filesystem_error([&] { rename(dir, otherDir); }, "rename", dir, otherDir)); @@ -3364,7 +3379,7 @@ void test_space() { const path file(dir / L"test_space_file.txt"sv); error_code ec; - create_directory(dir.native(), ec); + create_directory(dir, ec); EXPECT(good(ec)); create_file_containing(file, L"hello"); @@ -3643,7 +3658,7 @@ void test_create_directory() { } void test_create_dirs_and_remove_all() { - const test_temp_directory tempDir("create_dirs_and_remove_all"sv); + const test_temp_directory tempDir("create_directories-and-remove_all"sv); const path& r = tempDir.directoryPath; // test long path support diff --git a/tests/std/tests/VSO_0000000_path_stream_parameter/test.cpp b/tests/std/tests/VSO_0000000_path_stream_parameter/test.cpp index 53b9f6576f..41910ec0cc 100644 --- a/tests/std/tests/VSO_0000000_path_stream_parameter/test.cpp +++ b/tests/std/tests/VSO_0000000_path_stream_parameter/test.cpp @@ -18,7 +18,7 @@ int main() { error_code ec; { - const auto testDir = get_test_directory("path_stream_parameter"); + const auto testDir = get_experimental_test_directory("path_stream_parameter"); fs::create_directories(testDir, ec); assert(!ec); @@ -81,7 +81,7 @@ int main() { #if _HAS_CXX17 { - const auto testDir = get_new_test_directory("path_stream_parameter"); + const auto testDir = get_test_directory("path_stream_parameter"); fs::create_directories(testDir.native(), ec); assert(!ec); diff --git a/tests/tr1/include/temp_file_name.h b/tests/tr1/include/temp_file_name.h deleted file mode 100644 index 79991f56f2..0000000000 --- a/tests/tr1/include/temp_file_name.h +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - -#pragma once - -#include -#include - -[[nodiscard]] inline std::string temp_file_name() { - std::string ret{"temp_file_"}; - std::uniform_int_distribution dist{0, 15}; - std::random_device rd; - - for (int i = 0; i < 64; ++i) { // 64 hexits = 256 bits of entropy - ret.push_back("0123456789ABCDEF"[dist(rd)]); - } - - ret += ".tmp"; - - return ret; -} diff --git a/tests/tr1/tests/cstdio/test.cpp b/tests/tr1/tests/cstdio/test.cpp index e0bacf6c45..e59d5a783a 100644 --- a/tests/tr1/tests/cstdio/test.cpp +++ b/tests/tr1/tests/cstdio/test.cpp @@ -5,13 +5,14 @@ #define TEST_NAMEX "" #include "tdefs.h" -#include "temp_file_name.h" #include #include #include #include #include +#include + #undef clearerr // tested in stdio2.c #undef feof #undef ferror diff --git a/tests/tr1/tests/cwchar1/test.cpp b/tests/tr1/tests/cwchar1/test.cpp index d773b7b00a..b4449cd1e2 100644 --- a/tests/tr1/tests/cwchar1/test.cpp +++ b/tests/tr1/tests/cwchar1/test.cpp @@ -5,13 +5,14 @@ #define TEST_NAMEX ", part 1" #include "tdefs.h" -#include "temp_file_name.h" #include #include #include #include #include +#include + #pragma warning(disable : 4793) // function compiled as native diff --git a/tests/tr1/tests/fstream1/test.cpp b/tests/tr1/tests/fstream1/test.cpp index af558e99fd..680bd7e864 100644 --- a/tests/tr1/tests/fstream1/test.cpp +++ b/tests/tr1/tests/fstream1/test.cpp @@ -5,11 +5,12 @@ #define TEST_NAME ", part 1" #include "tdefs.h" -#include "temp_file_name.h" #include #include #include +#include + void test_main() { // test basic workings of char fstream definitions STD string tn_str = temp_file_name(); const char* tn = tn_str.c_str(); diff --git a/tests/tr1/tests/fstream2/test.cpp b/tests/tr1/tests/fstream2/test.cpp index b040145949..42efa9c12a 100644 --- a/tests/tr1/tests/fstream2/test.cpp +++ b/tests/tr1/tests/fstream2/test.cpp @@ -5,11 +5,12 @@ #define TEST_NAME ", part 2" #include "tdefs.h" -#include "temp_file_name.h" #include #include #include +#include + void test_main() { // test basic workings of wide fstream definitions const auto temp_name = temp_file_name(); const char* tn = temp_name.c_str();