Fix Test folders and registry cleaning logic

The test framework was not clearing out old registry values after successive
test runs. The new logic now correctly clears out the files and registry keys
after each run.

diff --git a/tests/framework/shim/shim_common.cpp b/tests/framework/shim/shim_common.cpp
index 227bc2c1c807894bd785c3a7499d5758be7ec73f..807ccd887fd5aa27e2adbfc593ee3270d7062308 100644 (file)
--- a/tests/framework/shim/shim_common.cpp
+++ b/tests/framework/shim/shim_common.cpp
@@ -94,11 +94,18 @@ HKEY create_key(HKEY key_root, const char* key_path) {
     return key;
 }
 
-void close_key(HKEY& key) {
+void close_key(HKEY key) {
     LSTATUS out = RegCloseKey(key);
     if (out != ERROR_SUCCESS) std::cerr << win_api_error_str(out) << " failed to close key " << key << "\n";
 }
 
+void delete_key(HKEY key, const char* key_path, bool report_failure = true) {
+    LSTATUS out = RegDeleteKeyA(key, key_path);
+    if (out != ERROR_SUCCESS)
+        if (report_failure)
+            std::cerr << win_api_error_str(out) << " failed to close key " << key << " with path " << key_path << "\n";
+}
+
 void setup_override_key(HKEY root_key, uint32_t random_base_path) {
     DWORD dDisposition{};
     LSTATUS out;
@@ -157,26 +164,6 @@ void remove_key_value(HKEY const& key, fs::path const& manifest_path) {
         std::cerr << win_api_error_str(out) << " failed to delete key value for " << manifest_path.str() << "\n";
 }
 
-void clear_key_values(HKEY const& key) {
-    DWORD dwNumValues, dwValueNameLen;
-
-    LSTATUS out = RegQueryInfoKey(key, 0, 0, 0, 0, 0, 0, &dwNumValues, &dwValueNameLen, 0, 0, 0);
-    if (out != ERROR_SUCCESS) {
-        std::cerr << win_api_error_str(out) << "couldn't query enum " << key << " values\n";
-        return;
-    }
-    std::string tchValName(dwValueNameLen + 1, '\0');
-    for (DWORD i = 0; i < dwNumValues; i++) {
-        DWORD length = static_cast<DWORD>(tchValName.size());
-        LPSTR lpstr = &tchValName[0];
-        out = RegEnumValue(key, i, lpstr, &length, 0, 0, 0, 0);
-        if (out != ERROR_SUCCESS) {
-            std::cerr << win_api_error_str(out) << "couldn't get enum value " << tchValName << "\n";
-            return;
-        }
-    }
-}
-
 uint32_t setup_override(DebugMode debug_mode) {
     uint32_t random_base_path = 0;
     std::random_device rd;
@@ -221,14 +208,16 @@ void clear_override(DebugMode debug_mode, uint32_t random_base_path) {
     }
 }
 void PlatformShim::reset(DebugMode debug_mode) {
-    KeyWrapper implicit_key{HKEY_LOCAL_MACHINE, "SOFTWARE\\Khronos\\Vulkan\\ImplicitLayers"};
-    KeyWrapper explicit_key{HKEY_LOCAL_MACHINE, "SOFTWARE\\Khronos\\Vulkan\\ExplicitLayers"};
-    KeyWrapper icd_key{HKEY_LOCAL_MACHINE, "SOFTWARE\\Khronos\\Vulkan\\Drivers"};
-    if (debug_mode != DebugMode::no_delete) {
-        clear_key_values(implicit_key);
-        clear_key_values(explicit_key);
-        clear_key_values(icd_key);
-    }
+    delete_key(HKEY_CURRENT_USER, "SOFTWARE\\Khronos\\Vulkan\\ImplicitLayers", false);
+    delete_key(HKEY_CURRENT_USER, "SOFTWARE\\Khronos\\Vulkan\\ExplicitLayers", false);
+
+    delete_key(HKEY_LOCAL_MACHINE, "SOFTWARE\\Khronos\\Vulkan\\ImplicitLayers", false);
+    delete_key(HKEY_LOCAL_MACHINE, "SOFTWARE\\Khronos\\Vulkan\\ExplicitLayers", false);
+    delete_key(HKEY_LOCAL_MACHINE, "SOFTWARE\\Khronos\\Vulkan\\Drivers", false);
+
+    delete_key(HKEY_LOCAL_MACHINE, "SOFTWARE\\WOW6432Node\\Khronos\\Vulkan\\Drivers", false);
+    delete_key(HKEY_LOCAL_MACHINE, "SOFTWARE\\WOW6432Node\\Khronos\\Vulkan\\ExplicitLayers", false);
+    delete_key(HKEY_LOCAL_MACHINE, "SOFTWARE\\WOW6432Node\\Khronos\\Vulkan\\ImplicitLayers", false);
 }
 
 void PlatformShim::set_path(ManifestCategory category, fs::path const& path) {}
@@ -270,11 +259,7 @@ void PlatformShim::add_CM_Device_ID(std::wstring const& id, fs::path const& icd_
     // add_key_value_string(id_key, "VulkanLayerName", layer_path.c_str());
 }
 
-HKEY GetRegistryKey() { return HKEY{}; }
-
 void PlatformShim::redirect_category(fs::path const& new_path, ManifestCategory search_category) {
-    // create_key(HKEY_LOCAL_MACHINE, "SYSTEM\\CurrentControlSet\\Control\\Class\\{4d36e968-e325-11ce-bfc1-08002be10318}\\0001");
-    // create_key(HKEY_LOCAL_MACHINE, "SYSTEM\\CurrentControlSet\\Control\\Class\\{4d36e968-e325-11ce-bfc1-08002be10318}\\0002");
     switch (search_category) {
         case (ManifestCategory::implicit_layer):
             create_key(HKEY_CURRENT_USER, "SOFTWARE\\Khronos\\Vulkan\\ImplicitLayers");

diff --git a/tests/framework/test_util.cpp b/tests/framework/test_util.cpp
index 923de83d744d130ccdd6dbc29d806322b3da4f39..193dd587a2df1da8baca283880fac37b51fde06b 100644 (file)
--- a/tests/framework/test_util.cpp
+++ b/tests/framework/test_util.cpp
@@ -363,11 +363,25 @@ int delete_folder(path const& folder) {
         // nothing to delete
         return 0;
     }
-    bool ret = RemoveDirectoryA(folder.c_str());
-    if (ret == 0) {
-        print_error_message(ERROR_REMOVEDIRECTORY_FAILED, "RemoveDirectoryA");
+    std::string search_path = folder.str() + "/*.*";
+    std::string s_p = folder.str() + "/";
+    WIN32_FIND_DATA fd;
+    HANDLE hFind = ::FindFirstFileA(search_path.c_str(), &fd);
+    if (hFind != INVALID_HANDLE_VALUE) {
+        do {
+            if (fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
+                if (!string_eq(fd.cFileName, ".") && !string_eq(fd.cFileName, "..")) {
+                    delete_folder(s_p + fd.cFileName);
+                }
+            } else {
+                std::string child_name = s_p + fd.cFileName;
+                DeleteFile(child_name.c_str());
+            }
+        } while (::FindNextFile(hFind, &fd));
+        ::FindClose(hFind);
+        _rmdir(folder.c_str());
     }
-    return ret;
+    return 0;
 #else
     DIR* dir = opendir(folder.c_str());
     if (!dir) {
@@ -403,10 +417,15 @@ FolderManager::FolderManager(path root_path, std::string name, DebugMode debug)
     create_folder(folder);
 }
 FolderManager::~FolderManager() {
-    for (auto& file : files) {
+    auto list_of_files_to_delete = files;
+    // remove(file) modifies the files variable, copy the list before deleting it
+    // Note: the allocation tests currently leak the loaded driver handles because in an OOM scenario the loader doesn't bother
+    // removing those. Since this is in an OOM situation, it is a low priority to fix. It does have the effect that Windows will
+    // be unable to delete the binaries that were leaked.
+    for (auto& file : list_of_files_to_delete) {
         if (debug >= DebugMode::log) std::cout << "Removing manifest " << file << " at " << (folder / file).str() << "\n";
         if (debug != DebugMode::no_delete) {
-            std::remove((folder / file).c_str());
+            remove(file);
         }
     }
     if (debug != DebugMode::no_delete) {
@@ -455,13 +474,17 @@ void FolderManager::remove(std::string const& name) {
     path out_path = folder / name;
     auto found = std::find(files.begin(), files.end(), name);
     if (found != files.end()) {
-        if (debug >= DebugMode::log) std::cout << "Removing manifest " << name << " at " << out_path.str() << "\n";
+        if (debug >= DebugMode::log) std::cout << "Removing file " << name << " at " << out_path.str() << "\n";
         if (debug != DebugMode::no_delete) {
-            std::remove(out_path.c_str());
+            int rc = std::remove(out_path.c_str());
+            if (rc != 0 && debug >= DebugMode::log) {
+                std::cerr << "Failed to remove file " << name << " at " << out_path.str() << "\n";
+            }
+
             files.erase(found);
         }
     } else {
-        if (debug >= DebugMode::log) std::cout << "Couldn't remove manifest " << name << " at " << out_path.str() << ".\n";
+        if (debug >= DebugMode::log) std::cout << "Couldn't remove file " << name << " at " << out_path.str() << ".\n";
     }
 }
 
@@ -471,6 +494,8 @@ path FolderManager::copy_file(path const& file, std::string const& new_name) {
     auto found = std::find(files.begin(), files.end(), new_name);
     if (found != files.end()) {
         if (debug >= DebugMode::log) std::cout << "File location already contains" << new_name << ". Is this a bug?\n";
+    } else if (file.str() == new_filepath.str()) {
+        if (debug >= DebugMode::log) std::cout << "Trying to copy " << new_name << " into itself. Is this a bug?\n";
     } else {
         if (debug >= DebugMode::log) std::cout << "Copying file" << file.str() << " to " << new_filepath.str() << "\n";
         files.emplace_back(new_name);
@@ -482,7 +507,7 @@ path FolderManager::copy_file(path const& file, std::string const& new_name) {
     }
     std::ofstream dst(new_filepath.str(), std::ios::binary);
     if (!dst) {
-        std::cerr << "Failed to create file " << file.str() << " for copying to\n";
+        std::cerr << "Failed to create file " << new_filepath.str() << " for copying to\n";
         return new_filepath;
     }
     dst << src.rdbuf();

diff --git a/tests/loader_testing_main.cpp b/tests/loader_testing_main.cpp
index c9e652ac5c06a99502af9bd28e760f3c6cec0873..a2aefd0075024ff94f555f9de72b70b0a126f836 100644 (file)
--- a/tests/loader_testing_main.cpp
+++ b/tests/loader_testing_main.cpp
@@ -70,8 +70,8 @@ int main(int argc, char** argv) {
     // Use an env-var to signal whether the override has been set up.
     uint32_t random_base_path = 0;
     std::string env_var{"size_large_enough"};
-    DWORD ret = GetEnvironmentVariable("VK_LOADER_TEST_REGISTRY_IS_SETUP", (LPSTR)env_var.c_str(), 256);
-    if (ret == 0) {
+    DWORD has_not_setup_tests = GetEnvironmentVariable("VK_LOADER_TEST_REGISTRY_IS_SETUP", (LPSTR)env_var.c_str(), 256);
+    if (has_not_setup_tests == 0) {
         random_base_path = setup_override(DebugMode::none);
         set_env_var("VK_LOADER_TEST_REGISTRY_IS_SETUP", "SETUP");
     }
@@ -80,7 +80,9 @@ int main(int argc, char** argv) {
     ::testing::InitGoogleTest(&argc, argv);
     int result = RUN_ALL_TESTS();
 #if defined(_WIN32)
-    clear_override(DebugMode::none, random_base_path);
+    if (has_not_setup_tests == 0) {
+        clear_override(DebugMode::none, random_base_path);
+    }
 #endif
     return result;
 }