Skip to content

Commit dbb1b37

Browse files
Cleanup tests ICD creation
Reformulate how tests create test ICD's to simplify and reduce clutter. Moves ManifestICD out of the TestCreateDetails and put it as a separate argument to make it easier to configure the Manifest & the configuration of the binary.
1 parent 2054164 commit dbb1b37

16 files changed

+815
-835
lines changed

tests/framework/test_environment.cpp

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -334,10 +334,15 @@ FrameworkEnvironment::~FrameworkEnvironment() {
334334
platform_shim->is_during_destruction = true;
335335
}
336336

337-
TestICD& FrameworkEnvironment::add_icd(TestICDDetails icd_details) noexcept {
337+
TestICD& FrameworkEnvironment::add_icd(std::filesystem::path const& path, ManifestOptions args, ManifestICD manifest) noexcept {
338+
manifest.set_lib_path(path);
339+
340+
if (args.json_name.empty()) {
341+
args.json_name = "test_icd.json";
342+
}
338343
size_t cur_icd_index = icds.size();
339344
fs::Folder* fs_ptr = &get_folder(ManifestLocation::driver);
340-
switch (icd_details.discovery_type) {
345+
switch (args.discovery_type) {
341346
case (ManifestDiscoveryType::env_var):
342347
case (ManifestDiscoveryType::add_env_var):
343348
fs_ptr = &get_folder(ManifestLocation::driver_env_var);
@@ -368,44 +373,44 @@ TestICD& FrameworkEnvironment::add_icd(TestICDDetails icd_details) noexcept {
368373
}
369374
auto& folder = *fs_ptr;
370375

371-
if (!icd_details.is_fake) {
372-
std::filesystem::path new_lib_name = icd_details.icd_manifest.lib_path.stem();
376+
if (!args.is_fake) {
377+
std::filesystem::path new_lib_name = manifest.lib_path.stem();
373378
new_lib_name += "_";
374379
new_lib_name += std::to_string(cur_icd_index);
375-
new_lib_name += icd_details.icd_manifest.lib_path.extension();
376-
auto new_driver_location = folder.copy_file(icd_details.icd_manifest.lib_path, new_lib_name);
380+
new_lib_name += manifest.lib_path.extension();
381+
auto new_driver_location = folder.copy_file(manifest.lib_path, new_lib_name);
377382

378383
#if TESTING_COMMON_UNIX_PLATFORMS
379-
if (icd_details.library_path_type == LibraryPathType::default_search_paths) {
384+
if (args.library_path_type == LibraryPathType::default_search_paths) {
380385
platform_shim->add_system_library(new_lib_name, new_driver_location);
381386
}
382387
#endif
383388
#if defined(WIN32)
384-
if (icd_details.library_path_type == LibraryPathType::default_search_paths) {
389+
if (args.library_path_type == LibraryPathType::default_search_paths) {
385390
SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_USER_DIRS);
386391
AddDllDirectory(new_driver_location.parent_path().native().c_str());
387392
}
388393
#endif
389394
icds.push_back(TestICDHandle(new_driver_location));
390395
icds.back().reset();
391-
if (icd_details.library_path_type == LibraryPathType::relative) {
392-
icd_details.icd_manifest.lib_path = std::filesystem::path(".") / new_lib_name;
393-
} else if (icd_details.library_path_type == LibraryPathType::default_search_paths) {
394-
icd_details.icd_manifest.lib_path = new_lib_name;
396+
if (args.library_path_type == LibraryPathType::relative) {
397+
manifest.lib_path = std::filesystem::path(".") / new_lib_name;
398+
} else if (args.library_path_type == LibraryPathType::default_search_paths) {
399+
manifest.lib_path = new_lib_name;
395400
} else {
396-
icd_details.icd_manifest.lib_path = new_driver_location;
401+
manifest.lib_path = new_driver_location;
397402
}
398403
}
399-
if (icd_details.discovery_type != ManifestDiscoveryType::none) {
400-
std::filesystem::path new_manifest_path = icd_details.json_name.stem();
401-
if (!icd_details.disable_icd_inc) {
404+
if (args.discovery_type != ManifestDiscoveryType::none) {
405+
std::filesystem::path new_manifest_path = args.json_name.stem();
406+
if (!args.disable_name_increment) {
402407
new_manifest_path += "_";
403408
new_manifest_path += std::to_string(cur_icd_index);
404409
}
405410
new_manifest_path += ".json";
406-
icds.back().manifest_path = folder.write_manifest(new_manifest_path, icd_details.icd_manifest.get_manifest_str());
411+
icds.back().manifest_path = folder.write_manifest(new_manifest_path, manifest.get_manifest_str());
407412
icds.back().shimmed_manifest_path = icds.back().manifest_path;
408-
switch (icd_details.discovery_type) {
413+
switch (args.discovery_type) {
409414
case (ManifestDiscoveryType::generic):
410415
#if defined(WIN32)
411416
platform_shim->add_manifest_to_registry(ManifestCategory::icd, icds.back().manifest_path);
@@ -424,14 +429,14 @@ TestICD& FrameworkEnvironment::add_icd(TestICDDetails icd_details) noexcept {
424429
#endif
425430
break;
426431
case (ManifestDiscoveryType::env_var):
427-
if (icd_details.is_dir) {
432+
if (args.is_dir) {
428433
env_var_vk_icd_filenames.add_to_list(folder.location());
429434
} else {
430435
env_var_vk_icd_filenames.add_to_list(folder.location() / new_manifest_path);
431436
}
432437
break;
433438
case (ManifestDiscoveryType::add_env_var):
434-
if (icd_details.is_dir) {
439+
if (args.is_dir) {
435440
add_env_var_vk_icd_filenames.add_to_list(folder.location());
436441
} else {
437442
add_env_var_vk_icd_filenames.add_to_list(folder.location() / new_manifest_path);

tests/framework/test_environment.h

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -436,15 +436,10 @@ struct TestBinaryHandle {
436436
using TestICDHandle = TestBinaryHandle<TestICD, GetTestICDFunc, GetNewTestICDFunc>;
437437
using TestLayerHandle = TestBinaryHandle<TestLayer, GetTestLayerFunc, GetNewTestLayerFunc>;
438438

439-
struct TestICDDetails {
440-
TestICDDetails(ManifestICD icd_manifest) noexcept : icd_manifest(icd_manifest) {}
441-
TestICDDetails(std::filesystem::path icd_binary_path, uint32_t api_version = VK_API_VERSION_1_0) noexcept {
442-
icd_manifest.set_lib_path(icd_binary_path).set_api_version(api_version);
443-
}
444-
BUILDER_VALUE(ManifestICD, icd_manifest);
445-
BUILDER_VALUE_WITH_DEFAULT(std::filesystem::path, json_name, "test_icd");
439+
struct ManifestOptions {
440+
BUILDER_VALUE(std::filesystem::path, json_name);
446441
// Uses the json_name without modification - default is to append _1 in the json file to distinguish drivers
447-
BUILDER_VALUE(bool, disable_icd_inc);
442+
BUILDER_VALUE(bool, disable_name_increment);
448443
BUILDER_VALUE_WITH_DEFAULT(ManifestDiscoveryType, discovery_type, ManifestDiscoveryType::generic);
449444
BUILDER_VALUE(bool, is_fake);
450445
// If discovery type is env-var, is_dir controls whether to use the path to the file or folder the manifest is in
@@ -487,7 +482,9 @@ struct FrameworkEnvironment {
487482
FrameworkEnvironment(const FrameworkEnvironment&) = delete;
488483
FrameworkEnvironment& operator=(const FrameworkEnvironment&) = delete;
489484

490-
TestICD& add_icd(TestICDDetails icd_details) noexcept;
485+
TestICD& add_icd(std::filesystem::path const& path, ManifestOptions args = ManifestOptions{},
486+
ManifestICD manifest = ManifestICD{}) noexcept;
487+
491488
void add_implicit_layer(ManifestLayer layer_manifest, const std::string& json_name) noexcept;
492489
void add_implicit_layer(TestLayerDetails layer_details) noexcept;
493490
void add_explicit_layer(ManifestLayer layer_manifest, const std::string& json_name) noexcept;

tests/framework/util/manifest_builders.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ struct ManifestVersion {
5050
// ManifestICD builder
5151
struct ManifestICD {
5252
BUILDER_VALUE(ManifestVersion, file_format_version)
53-
BUILDER_VALUE(uint32_t, api_version)
53+
BUILDER_VALUE_WITH_DEFAULT(uint32_t, api_version, VK_API_VERSION_1_0)
5454
BUILDER_VALUE(std::filesystem::path, lib_path)
5555
BUILDER_VALUE(bool, is_portability_driver)
5656
BUILDER_VALUE(std::string, library_arch)

tests/loader_alloc_callback_tests.cpp

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@
2525
* Author: Charles Giessen <[email protected]>
2626
*/
2727

28+
#include "manifest_builders.h"
2829
#include "test_environment.h"
2930

30-
#include <fstream>
3131
#include <mutex>
3232

3333
struct MemoryTrackerSettings {
@@ -184,7 +184,7 @@ class MemoryTracker {
184184
// a CreateInstance/DestroyInstance call pair.
185185
TEST(Allocation, Instance) {
186186
FrameworkEnvironment env{};
187-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
187+
env.add_icd(TEST_ICD_PATH_VERSION_2);
188188

189189
MemoryTracker tracker;
190190
{
@@ -198,7 +198,7 @@ TEST(Allocation, Instance) {
198198
// a CreateInstance/DestroyInstance call pair with a call to GetInstanceProcAddr.
199199
TEST(Allocation, GetInstanceProcAddr) {
200200
FrameworkEnvironment env{};
201-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
201+
env.add_icd(TEST_ICD_PATH_VERSION_2);
202202

203203
MemoryTracker tracker;
204204
{
@@ -216,7 +216,7 @@ TEST(Allocation, GetInstanceProcAddr) {
216216
// a vkEnumeratePhysicalDevices call pair.
217217
TEST(Allocation, EnumeratePhysicalDevices) {
218218
FrameworkEnvironment env{};
219-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).add_physical_device("physical_device_0");
219+
env.add_icd(TEST_ICD_PATH_VERSION_2).add_physical_device("physical_device_0");
220220

221221
MemoryTracker tracker;
222222
{
@@ -239,7 +239,7 @@ TEST(Allocation, EnumeratePhysicalDevices) {
239239
// allocators used on both the instance and device.
240240
TEST(Allocation, InstanceAndDevice) {
241241
FrameworkEnvironment env{};
242-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2))
242+
env.add_icd(TEST_ICD_PATH_VERSION_2)
243243
.add_physical_device(PhysicalDevice{"physical_device_0"}
244244
.add_queue_family_properties({{VK_QUEUE_GRAPHICS_BIT, 1, 0, {1, 1, 1}}, false})
245245
.finish());
@@ -288,7 +288,7 @@ TEST(Allocation, InstanceAndDevice) {
288288
// allocators used on only the instance and not the device.
289289
TEST(Allocation, InstanceButNotDevice) {
290290
FrameworkEnvironment env{};
291-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2))
291+
env.add_icd(TEST_ICD_PATH_VERSION_2)
292292
.add_physical_device(PhysicalDevice{"physical_device_0"}
293293
.add_queue_family_properties({{VK_QUEUE_GRAPHICS_BIT, 1, 0, {1, 1, 1}}, false})
294294
.finish());
@@ -337,7 +337,7 @@ TEST(Allocation, InstanceButNotDevice) {
337337
// allocators used on only the device and not the instance.
338338
TEST(Allocation, DeviceButNotInstance) {
339339
FrameworkEnvironment env{};
340-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2))
340+
env.add_icd(TEST_ICD_PATH_VERSION_2)
341341
.add_physical_device(PhysicalDevice{"physical_device_0"}
342342
.add_queue_family_properties({{VK_QUEUE_GRAPHICS_BIT, 1, 0, {1, 1, 1}}, false})
343343
.finish());
@@ -394,7 +394,7 @@ TEST(Allocation, DeviceButNotInstance) {
394394
// one of the out-of-memory conditions trigger.
395395
TEST(Allocation, CreateInstanceIntentionalAllocFail) {
396396
FrameworkEnvironment env{FrameworkSettings{}.set_log_filter("error,warn")};
397-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
397+
env.add_icd(TEST_ICD_PATH_VERSION_2);
398398

399399
const char* layer_name = "VK_LAYER_implicit";
400400
env.add_implicit_layer(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
@@ -424,7 +424,7 @@ TEST(Allocation, CreateInstanceIntentionalAllocFail) {
424424
// one of the out-of-memory conditions trigger and there are invalid jsons in the same folder
425425
TEST(Allocation, CreateInstanceIntentionalAllocFailInvalidManifests) {
426426
FrameworkEnvironment env{FrameworkSettings{}.set_log_filter("error,warn")};
427-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
427+
env.add_icd(TEST_ICD_PATH_VERSION_2);
428428

429429
std::vector<std::string> invalid_jsons;
430430
invalid_jsons.push_back(",");
@@ -472,7 +472,7 @@ TEST(Allocation, CreateInstanceIntentionalAllocFailInvalidManifests) {
472472
// one of the out-of-memory conditions trigger.
473473
TEST(Allocation, CreateSurfaceIntentionalAllocFail) {
474474
FrameworkEnvironment env{FrameworkSettings{}.set_log_filter("error,warn")};
475-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).setup_WSI();
475+
env.add_icd(TEST_ICD_PATH_VERSION_2).setup_WSI();
476476

477477
const char* layer_name = "VK_LAYER_implicit";
478478
env.add_implicit_layer(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
@@ -517,7 +517,7 @@ TEST(Allocation, CreateSurfaceIntentionalAllocFail) {
517517
// one of the out-of-memory conditions trigger.
518518
TEST(Allocation, CreateInstanceIntentionalAllocFailWithSettingsFilePresent) {
519519
FrameworkEnvironment env{FrameworkSettings{}.set_log_filter("error,warn")};
520-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
520+
env.add_icd(TEST_ICD_PATH_VERSION_2);
521521

522522
const char* layer_name = "VK_LAYER_implicit";
523523
env.add_implicit_layer(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
@@ -554,7 +554,7 @@ TEST(Allocation, CreateInstanceIntentionalAllocFailWithSettingsFilePresent) {
554554
// one of the out-of-memory conditions trigger.
555555
TEST(Allocation, CreateSurfaceIntentionalAllocFailWithSettingsFilePresent) {
556556
FrameworkEnvironment env{FrameworkSettings{}.set_log_filter("error,warn")};
557-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).setup_WSI();
557+
env.add_icd(TEST_ICD_PATH_VERSION_2).setup_WSI();
558558

559559
const char* layer_name = "VK_LAYER_implicit";
560560
env.add_implicit_layer(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
@@ -605,7 +605,7 @@ TEST(Allocation, CreateSurfaceIntentionalAllocFailWithSettingsFilePresent) {
605605
// one of the out-of-memory conditions trigger.
606606
TEST(Allocation, DriverEnvVarIntentionalAllocFail) {
607607
FrameworkEnvironment env{FrameworkSettings{}.set_log_filter("error,warn")};
608-
env.add_icd(TestICDDetails{TEST_ICD_PATH_VERSION_2}.set_discovery_type(ManifestDiscoveryType::env_var));
608+
env.add_icd(TEST_ICD_PATH_VERSION_2, ManifestOptions{}.set_discovery_type(ManifestDiscoveryType::env_var));
609609

610610
const char* layer_name = "VK_LAYER_implicit";
611611
env.add_implicit_layer(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
@@ -639,7 +639,7 @@ TEST(Allocation, DriverEnvVarIntentionalAllocFail) {
639639
// may fail.
640640
TEST(Allocation, CreateDeviceIntentionalAllocFail) {
641641
FrameworkEnvironment env{FrameworkSettings{}.set_log_filter("error,warn")};
642-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2))
642+
env.add_icd(TEST_ICD_PATH_VERSION_2)
643643
.add_physical_device(PhysicalDevice{"physical_device_0"}
644644
.add_queue_family_properties({{VK_QUEUE_GRAPHICS_BIT, 1, 0, {1, 1, 1}}, false})
645645
.finish())
@@ -709,19 +709,18 @@ TEST(Allocation, CreateInstanceDeviceIntentionalAllocFail) {
709709
uint32_t num_physical_devices = 4;
710710
uint32_t num_implicit_layers = 3;
711711
for (uint32_t i = 0; i < num_physical_devices; i++) {
712-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)
713-
.icd_manifest.set_is_portability_driver(false)
714-
.set_library_arch(sizeof(void*) == 8 ? "64" : "32"))
712+
env.add_icd(TEST_ICD_PATH_VERSION_2, {},
713+
ManifestICD{}.set_is_portability_driver(false).set_library_arch(sizeof(void*) == 8 ? "64" : "32"))
715714
.set_icd_api_version(VK_API_VERSION_1_1)
716715
.add_instance_extension("VK_KHR_get_physical_device_properties2")
717716
.add_and_get_physical_device("physical_device_0")
718717
.add_queue_family_properties({{VK_QUEUE_GRAPHICS_BIT, 1, 0, {1, 1, 1}}, false})
719718
.add_extensions({"VK_EXT_one", "VK_EXT_two", "VK_EXT_three", "VK_EXT_four", "VK_EXT_five"});
720719
}
721720

722-
env.add_icd(TestICDDetails(CURRENT_PLATFORM_DUMMY_BINARY_WRONG_TYPE).set_is_fake(true));
721+
env.add_icd(CURRENT_PLATFORM_DUMMY_BINARY_WRONG_TYPE, ManifestOptions{}.set_is_fake(true));
723722

724-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_7).set_discovery_type(ManifestDiscoveryType::none));
723+
env.add_icd(TEST_ICD_PATH_VERSION_7, ManifestOptions{}.set_discovery_type(ManifestDiscoveryType::none));
725724

726725
VkDirectDriverLoadingInfoLUNARG ddl_info{};
727726
ddl_info.sType = VK_STRUCTURE_TYPE_DIRECT_DRIVER_LOADING_INFO_LUNARG;
@@ -827,8 +826,8 @@ TEST(Allocation, CreateInstanceDeviceIntentionalAllocFail) {
827826
// an incompatible driver exists
828827
TEST(TryLoadWrongBinaries, CreateInstanceIntentionalAllocFail) {
829828
FrameworkEnvironment env{FrameworkSettings{}.set_log_filter("error,warn")};
830-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
831-
env.add_icd(TestICDDetails(CURRENT_PLATFORM_DUMMY_BINARY_WRONG_TYPE).set_is_fake(true));
829+
env.add_icd(TEST_ICD_PATH_VERSION_2);
830+
env.add_icd(CURRENT_PLATFORM_DUMMY_BINARY_WRONG_TYPE, ManifestOptions{}.set_is_fake(true));
832831

833832
const char* layer_name = "VK_LAYER_implicit";
834833
env.add_implicit_layer(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
@@ -859,7 +858,7 @@ TEST(TryLoadWrongBinaries, CreateInstanceIntentionalAllocFail) {
859858
// leak memory if one of the out-of-memory conditions trigger.
860859
TEST(Allocation, EnumeratePhysicalDevicesIntentionalAllocFail) {
861860
FrameworkEnvironment env{FrameworkSettings{}.set_log_filter("error,warn")};
862-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
861+
env.add_icd(TEST_ICD_PATH_VERSION_2);
863862

864863
const char* layer_name = "VK_LAYER_implicit";
865864
env.add_implicit_layer(ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{}
@@ -970,8 +969,8 @@ TEST(Allocation, EnumeratePhysicalDevicesIntentionalAllocFail) {
970969
// leak memory if one of the out-of-memory conditions trigger.
971970
TEST(Allocation, CreateInstanceDeviceWithDXGIDriverIntentionalAllocFail) {
972971
FrameworkEnvironment env{FrameworkSettings{}.set_log_filter("error,warn")};
973-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_6).set_discovery_type(ManifestDiscoveryType::null_dir));
974-
env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2));
972+
env.add_icd(TEST_ICD_PATH_VERSION_6, ManifestOptions{}.set_discovery_type(ManifestDiscoveryType::null_dir));
973+
env.add_icd(TEST_ICD_PATH_VERSION_2);
975974

976975
for (uint32_t i = 0; i < 2; i++) {
977976
auto& driver = env.get_test_icd(i);

0 commit comments

Comments
 (0)