Fix EnvVarWrapper overwriting values
authorCharles Giessen <charles@lunarg.com>
Sun, 11 Jun 2023 00:00:56 +0000 (18:00 -0600)
committerCharles Giessen <46324611+charles-lunarg@users.noreply.github.com>
Tue, 13 Jun 2023 03:37:40 +0000 (21:37 -0600)
When EnvVarWrapper gets created, it clobbers the existing value of the
environment variable if it exists. When running multiple tests in a row
inside of a single process, the values set by the loader_testing_main.cpp
should be what every test is initialized with. To make that happen,
EnvVarWrapper now will save the existing value if it exists, then re-apply
it during its destructor.

tests/framework/test_util.h
tests/loader_testing_main.cpp

index 1826c03b808e4aa37ad2923a4e92085ba0ecfb61..28bc2fd9d78438820c7847aeed7c485eac6bf4a4 100644 (file)
 
 #include "json_writer.h"
 
+// get_env_var() - returns a std::string of `name`. if report_failure is true, then it will log to stderr that it didn't find the
+//     env-var
+// NOTE: This is only intended for test framework code, all test code MUST use EnvVarWrapper
+std::string get_env_var(std::string const& name, bool report_failure = true);
+
 /*
  * Wrapper around Environment Variables with common operations
  * Since Environment Variables leak between tests, there needs to be RAII code to remove them during test cleanup
 // Wrapper to set & remove env-vars automatically
 struct EnvVarWrapper {
     // Constructor which unsets the env-var
-    EnvVarWrapper(std::string const& name) noexcept : name(name) { remove_env_var(); }
+    EnvVarWrapper(std::string const& name) noexcept : name(name) {
+        initial_value = get_env_var(name, false);
+        remove_env_var();
+    }
     // Constructor which set the env-var to the specified value
-    EnvVarWrapper(std::string const& name, std::string const& value) noexcept : name(name), cur_value(value) { set_env_var(); }
-    ~EnvVarWrapper() noexcept { remove_env_var(); }
+    EnvVarWrapper(std::string const& name, std::string const& value) noexcept : name(name), cur_value(value) {
+        initial_value = get_env_var(name, false);
+        set_env_var();
+    }
+    ~EnvVarWrapper() noexcept {
+        remove_env_var();
+        if (!initial_value.empty()) {
+            set_new_value(initial_value);
+        }
+    }
 
     // delete copy operators
     EnvVarWrapper(const EnvVarWrapper&) = delete;
@@ -140,6 +156,7 @@ struct EnvVarWrapper {
    private:
     std::string name;
     std::string cur_value;
+    std::string initial_value;
 
     void set_env_var();
     void remove_env_var() const;
@@ -152,11 +169,6 @@ struct EnvVarWrapper {
 #endif
 };
 
-// get_env_var() - returns a std::string of `name`. if report_failure is true, then it will log to stderr that it didn't find the
-//     env-var
-// NOTE: This is only intended for test framework code, all test code MUST use EnvVarWrapper
-std::string get_env_var(std::string const& name, bool report_failure = true);
-
 // Windows specific error handling logic
 #if defined(WIN32)
 const long ERROR_SETENV_FAILED = 10543;           // chosen at random, attempts to not conflict
index 6b5a44082e4c9dda0f8044a0f263063268aff8a0..46f006bc5fc6333688a1d3d7d1242b4f66307a18 100644 (file)
@@ -80,18 +80,6 @@ int main(int argc, char** argv) {
     EnvVarWrapper vk_loader_layers_disable_env_var{"VK_LOADER_LAYERS_DISABLE"};
     EnvVarWrapper vk_loader_debug_env_var{"VK_LOADER_DEBUG"};
     EnvVarWrapper vk_loader_disable_inst_ext_filter_env_var{"VK_LOADER_DISABLE_INST_EXT_FILTER"};
-    vk_icd_filenames_env_var.remove_value();
-    vk_driver_files_env_var.remove_value();
-    vk_add_driver_files_env_var.remove_value();
-    vk_layer_path_env_var.remove_value();
-    vk_add_layer_path_env_var.remove_value();
-    vk_instance_layers_env_var.remove_value();
-    vk_loader_drivers_select_env_var.remove_value();
-    vk_loader_drivers_disable_env_var.remove_value();
-    vk_loader_layers_enable_env_var.remove_value();
-    vk_loader_layers_disable_env_var.remove_value();
-    vk_loader_debug_env_var.remove_value();
-    vk_loader_disable_inst_ext_filter_env_var.remove_value();
 
 #if COMMON_UNIX_PLATFORMS
     // Set only one of the 4 XDG variables to /etc, let everything else be empty