Skip to content

Commit 612b10b

Browse files
charles-lunargcharles-lunarg
committed
Remove redundant bool in tests ManifestOptions
disable_name_increment was used to stop the test framework from adding _X to the manifest file name, so tests can test against the exact file name. Since tests have to specify a name, it is possible to infer the intent by just checking to see if json_name has been set. If it hasn't, "test_icd_x.json" is used, where x is used to distinguish between created ICD's. This makes writing tests easier and less error prone.
1 parent 5bcc1f2 commit 612b10b

File tree

4 files changed

+28
-35
lines changed

4 files changed

+28
-35
lines changed

‎tests/framework/test_environment.cpp‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -339,8 +339,9 @@ TestICD& FrameworkEnvironment::add_icd(std::filesystem::path const& path, Manife
339339
manifest.set_lib_path(path);
340340

341341
if (args.json_name.empty()) {
342-
args.json_name = "test_icd.json";
342+
args.json_name = "test_icd_" + std::to_string(icds.size()) + ".json";
343343
}
344+
344345
size_t cur_icd_index = icds.size();
345346
fs::Folder* fs_ptr = &get_folder(ManifestLocation::driver);
346347
switch (args.discovery_type) {
@@ -403,44 +404,37 @@ TestICD& FrameworkEnvironment::add_icd(std::filesystem::path const& path, Manife
403404
}
404405
}
405406
if (args.discovery_type != ManifestDiscoveryType::none) {
406-
std::filesystem::path new_manifest_path = args.json_name.stem();
407-
if (!args.disable_name_increment) {
408-
new_manifest_path += "_";
409-
new_manifest_path += std::to_string(cur_icd_index);
410-
}
411-
new_manifest_path += ".json";
412-
icds.back().manifest_path = folder.write_manifest(new_manifest_path, manifest.get_manifest_str());
407+
icds.back().manifest_path = folder.write_manifest(args.json_name, manifest.get_manifest_str());
413408
icds.back().shimmed_manifest_path = icds.back().manifest_path;
414409
switch (args.discovery_type) {
415410
case (ManifestDiscoveryType::generic):
416411
#if defined(WIN32)
417412
platform_shim->add_manifest_to_registry(ManifestCategory::icd, icds.back().manifest_path);
418413
#elif TESTING_COMMON_UNIX_PLATFORMS
419414
icds.back().shimmed_manifest_path =
420-
file_system_manager.get_path_redirect_by_manifest_location(ManifestLocation::driver) / new_manifest_path;
415+
file_system_manager.get_path_redirect_by_manifest_location(ManifestLocation::driver) / args.json_name;
421416
#endif
422417
break;
423418
case (ManifestDiscoveryType::unsecured_generic):
424419
#if defined(WIN32)
425420
platform_shim->add_unsecured_manifest_to_registry(ManifestCategory::icd, icds.back().manifest_path);
426421
#elif TESTING_COMMON_UNIX_PLATFORMS
427422
icds.back().shimmed_manifest_path =
428-
file_system_manager.get_path_redirect_by_manifest_location(ManifestLocation::unsecured_driver) /
429-
new_manifest_path;
423+
file_system_manager.get_path_redirect_by_manifest_location(ManifestLocation::unsecured_driver) / args.json_name;
430424
#endif
431425
break;
432426
case (ManifestDiscoveryType::env_var):
433427
if (args.is_dir) {
434428
env_var_vk_icd_filenames.add_to_list(folder.location());
435429
} else {
436-
env_var_vk_icd_filenames.add_to_list(folder.location() / new_manifest_path);
430+
env_var_vk_icd_filenames.add_to_list(folder.location() / args.json_name);
437431
}
438432
break;
439433
case (ManifestDiscoveryType::add_env_var):
440434
if (args.is_dir) {
441435
add_env_var_vk_icd_filenames.add_to_list(folder.location());
442436
} else {
443-
add_env_var_vk_icd_filenames.add_to_list(folder.location() / new_manifest_path);
437+
add_env_var_vk_icd_filenames.add_to_list(folder.location() / args.json_name);
444438
}
445439
break;
446440
break;

‎tests/framework/test_environment.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -438,8 +438,6 @@ using TestLayerHandle = TestBinaryHandle<TestLayer, GetTestLayerFunc, GetNewTest
438438

439439
struct ManifestOptions {
440440
BUILDER_VALUE(std::filesystem::path, json_name);
441-
// Uses the json_name without modification - default is to append _1 in the json file to distinguish drivers
442-
BUILDER_VALUE(bool, disable_name_increment);
443441
BUILDER_VALUE_WITH_DEFAULT(ManifestDiscoveryType, discovery_type, ManifestDiscoveryType::generic);
444442
BUILDER_VALUE(bool, is_fake);
445443
// If discovery type is env-var, is_dir controls whether to use the path to the file or folder the manifest is in

‎tests/loader_envvar_tests.cpp‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,10 @@ TEST(EnvVarICDOverrideSetup, version_2_negotiate_interface_version_and_icd_gipa)
8181
// export vk_icdNegotiateLoaderICDInterfaceVersion and vk_icdGetInstanceProcAddr
8282
TEST(EnvVarICDOverrideSetup, version_2_negotiate_interface_version_and_icd_gipa_unicode) {
8383
FrameworkEnvironment env{};
84-
env.add_icd(widen(TEST_ICD_PATH_VERSION_2_UNICODE), ManifestOptions{}
85-
.set_discovery_type(ManifestDiscoveryType::env_var)
86-
.set_json_name(widen(TEST_JSON_NAME_VERSION_2_UNICODE)));
84+
env.add_icd(widen(TEST_ICD_PATH_VERSION_2_UNICODE),
85+
ManifestOptions{}
86+
.set_discovery_type(ManifestDiscoveryType::env_var)
87+
.set_json_name(widen(std::string(TEST_JSON_NAME_VERSION_2_UNICODE) + ".json")));
8788

8889
InstWrapper inst{env.vulkan_functions};
8990
inst.CheckCreate();
@@ -564,10 +565,10 @@ TEST(EnvVarICDOverrideSetup, FilterSelectDriver) {
564565
FrameworkEnvironment env{};
565566
EnvVarWrapper filter_select_env_var{"VK_LOADER_DRIVERS_SELECT"};
566567

567-
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_disable_name_increment(true).set_json_name("ABC_ICD"));
568-
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_disable_name_increment(true).set_json_name("BCD_ICD"),
568+
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_json_name("ABC_ICD.json"));
569+
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_json_name("BCD_ICD.json"),
569570
ManifestICD{}.set_api_version(VK_API_VERSION_1_2));
570-
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_disable_name_increment(true).set_json_name("CDE_ICD"),
571+
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_json_name("CDE_ICD.json"),
571572
ManifestICD{}.set_api_version(VK_API_VERSION_1_3));
572573

573574
InstWrapper inst1{env.vulkan_functions};
@@ -716,10 +717,10 @@ TEST(EnvVarICDOverrideSetup, FilterDisableDriver) {
716717
FrameworkEnvironment env{};
717718
EnvVarWrapper filter_disable_env_var{"VK_LOADER_DRIVERS_DISABLE"};
718719

719-
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_disable_name_increment(true).set_json_name("ABC_ICD"));
720-
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_disable_name_increment(true).set_json_name("BCD_ICD"),
720+
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_json_name("ABC_ICD.json"));
721+
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_json_name("BCD_ICD.json"),
721722
ManifestICD{}.set_api_version(VK_API_VERSION_1_2));
722-
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_disable_name_increment(true).set_json_name("CDE_ICD"),
723+
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_json_name("CDE_ICD.json"),
723724
ManifestICD{}.set_api_version(VK_API_VERSION_1_3));
724725

725726
InstWrapper inst1{env.vulkan_functions};
@@ -852,10 +853,10 @@ TEST(EnvVarICDOverrideSetup, FilterSelectAndDisableDriver) {
852853
EnvVarWrapper filter_disable_env_var{"VK_LOADER_DRIVERS_DISABLE"};
853854
EnvVarWrapper filter_select_env_var{"VK_LOADER_DRIVERS_SELECT"};
854855

855-
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_disable_name_increment(true).set_json_name("ABC_ICD"));
856-
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_disable_name_increment(true).set_json_name("BCD_ICD"),
856+
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_json_name("ABC_ICD.json"));
857+
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_json_name("BCD_ICD.json"),
857858
ManifestICD{}.set_api_version(VK_API_VERSION_1_2));
858-
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_disable_name_increment(true).set_json_name("CDE_ICD"),
859+
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_json_name("CDE_ICD.json"),
859860
ManifestICD{}.set_api_version(VK_API_VERSION_1_3));
860861

861862
InstWrapper inst1{env.vulkan_functions};

‎tests/loader_version_tests.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,8 +1100,8 @@ TEST(DirectDriverLoading, InclusiveWithFilterSelect) {
11001100

11011101
EnvVarWrapper driver_filter_select_env_var{"VK_LOADER_DRIVERS_SELECT", "normal_driver.json"};
11021102

1103-
normal_drivers.emplace_back(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA,
1104-
ManifestOptions{}.set_json_name("normal_driver.json").set_disable_name_increment(true), 8, true);
1103+
normal_drivers.emplace_back(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA, ManifestOptions{}.set_json_name("normal_driver.json"), 8,
1104+
true);
11051105
direct_drivers.emplace_back(TEST_ICD_PATH_VERSION_7, ManifestOptions{}.set_discovery_type(ManifestDiscoveryType::none), 9,
11061106
true);
11071107

@@ -1115,8 +1115,8 @@ TEST(DirectDriverLoading, ExclusiveWithFilterSelect) {
11151115

11161116
EnvVarWrapper driver_filter_select_env_var{"VK_LOADER_DRIVERS_SELECT", "normal_driver.json"};
11171117

1118-
normal_drivers.emplace_back(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA,
1119-
ManifestOptions{}.set_json_name("normal_driver.json").set_disable_name_increment(true), 10, false);
1118+
normal_drivers.emplace_back(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA, ManifestOptions{}.set_json_name("normal_driver.json"), 10,
1119+
false);
11201120
direct_drivers.emplace_back(TEST_ICD_PATH_VERSION_7, ManifestOptions{}.set_discovery_type(ManifestDiscoveryType::none), 11,
11211121
true);
11221122

@@ -1130,8 +1130,8 @@ TEST(DirectDriverLoading, InclusiveWithFilterDisable) {
11301130

11311131
EnvVarWrapper driver_filter_disable_env_var{"VK_LOADER_DRIVERS_DISABLE", "normal_driver.json"};
11321132

1133-
normal_drivers.emplace_back(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA,
1134-
ManifestOptions{}.set_json_name("normal_driver.json").set_disable_name_increment(true), 12, false);
1133+
normal_drivers.emplace_back(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA, ManifestOptions{}.set_json_name("normal_driver.json"), 12,
1134+
false);
11351135
direct_drivers.emplace_back(TEST_ICD_PATH_VERSION_7, ManifestOptions{}.set_discovery_type(ManifestDiscoveryType::none), 13,
11361136
true);
11371137
ASSERT_NO_FATAL_FAILURE(CheckDirectDriverLoading(env, normal_drivers, direct_drivers, false));
@@ -1144,8 +1144,8 @@ TEST(DirectDriverLoading, ExclusiveWithFilterDisable) {
11441144

11451145
EnvVarWrapper driver_filter_disable_env_var{"VK_LOADER_DRIVERS_DISABLE", "normal_driver.json"};
11461146

1147-
normal_drivers.emplace_back(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA,
1148-
ManifestOptions{}.set_json_name("normal_driver.json").set_disable_name_increment(true), 14, false);
1147+
normal_drivers.emplace_back(TEST_ICD_PATH_VERSION_2_EXPORT_ICD_GPDPA, ManifestOptions{}.set_json_name("normal_driver.json"), 14,
1148+
false);
11491149
direct_drivers.emplace_back(TEST_ICD_PATH_VERSION_7, ManifestOptions{}.set_discovery_type(ManifestDiscoveryType::none), 15,
11501150
true);
11511151
ASSERT_NO_FATAL_FAILURE(CheckDirectDriverLoading(env, normal_drivers, direct_drivers, true));

0 commit comments

Comments
 (0)