Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

vulkan: Remove redundant negative testing #2078

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+0 −134
Open

vulkan: Remove redundant negative testing #2078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 0 additions & 52 deletions test_conformance/vulkan/test_vulkan_api_consistency.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,27 +323,6 @@ int test_consistency_external_image(cl_device_id deviceID, cl_context _context,
test_error(errNum, "Unable to create Image with Properties");
image.reset();

// Passing image_format as NULL
image = clCreateImageWithProperties(context, extMemProperties.data(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not related to semaphore negative tests, but for image import negative testing and should be retained, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These failure conditions are already described in the OpenCL base specification. If coverage is missing, they should be added to one of the core tests, such as API or basic. The exception is if cl_khr_external_memory has a special provision for image_format or image_desc that I am forgetting.

CL_MEM_READ_WRITE, NULL, &image_desc,
NULL, &errNum);
test_failure_error(errNum, CL_INVALID_IMAGE_FORMAT_DESCRIPTOR,
"Image creation must fail with "
"CL_INVALID_IMAGE_FORMAT_DESCRIPTOR"
"when image desc passed as NULL");

image.reset();

// Passing image_desc as NULL
image = clCreateImageWithProperties(context, extMemProperties.data(),
CL_MEM_READ_WRITE, &img_format, NULL,
NULL, &errNum);
test_failure_error(errNum, CL_INVALID_IMAGE_DESCRIPTOR,
"Image creation must fail with "
"CL_INVALID_IMAGE_DESCRIPTOR "
"when image desc passed as NULL");
image.reset();

return TEST_PASS;
}

Expand Down Expand Up @@ -479,31 +458,6 @@ int test_consistency_external_semaphore(cl_device_id deviceID,
sema_props1.push_back(0);
sema_props2.push_back(0);

// Pass NULL properties
cl_semaphore_khr cl_ext_semaphore =
clCreateSemaphoreWithPropertiesKHRptr(context, NULL, &errNum);
test_failure_error(errNum, CL_INVALID_VALUE,
"Semaphore creation must fail with CL_INVALID_VALUE "
" when properties are passed as NULL");


// Pass invalid semaphore object to wait
errNum =
clEnqueueWaitSemaphoresKHRptr(cmd_queue, 1, NULL, NULL, 0, NULL, NULL);
test_failure_error(errNum, CL_INVALID_VALUE,
"clEnqueueWaitSemaphoresKHR fails with CL_INVALID_VALUE "
"when invalid semaphore object is passed");


// Pass invalid semaphore object to signal
errNum = clEnqueueSignalSemaphoresKHRptr(cmd_queue, 1, NULL, NULL, 0, NULL,
NULL);
test_failure_error(
errNum, CL_INVALID_VALUE,
"clEnqueueSignalSemaphoresKHR fails with CL_INVALID_VALUE"
"when invalid semaphore object is passed");


// Create two semaphore objects
clVk2Clsemaphore = clCreateSemaphoreWithPropertiesKHRptr(
context, sema_props1.data(), &errNum);
Expand All @@ -515,12 +469,6 @@ int test_consistency_external_semaphore(cl_device_id deviceID,
test_error(errNum,
"Unable to create semaphore with valid semaphore properties");

// Pass invalid object to release call
errNum = clReleaseSemaphoreKHRptr(NULL);
test_failure_error(errNum, CL_INVALID_VALUE,
"clReleaseSemaphoreKHRptr fails with "
"CL_INVALID_VALUE when NULL semaphore object is passed");

// Release both semaphore objects
errNum = clReleaseSemaphoreKHRptr(clVk2Clsemaphore);
test_error(errNum, "clReleaseSemaphoreKHRptr failed");
Expand Down
41 changes: 0 additions & 41 deletions test_conformance/vulkan/test_vulkan_api_consistency_for_1dimages.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,47 +158,6 @@ int test_consistency_external_for_1dimage(cl_device_id deviceID,
test_error(errNum, "Unable to create Image with Properties");
image.reset();

// Passing properties, image_desc and image_format all as NULL
image = clCreateImageWithProperties(context, NULL, CL_MEM_READ_WRITE, NULL,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be retained for the same reason I mentioned above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case does not make sense for this extension, there is no information being passed that assosciates clCreateImageWithProperties and cl_khr_external_memory.

If we believe this is important coverage, it would be a better fit in api or basic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer we retain the coverage here until we add it to more appropriate place rather than removing it right away.
We can either add the same to more appropriate tests or move these tests to more appropriate place in a subsequent PR.

NULL, NULL, &errNum);
test_failure_error(
errNum, CL_INVALID_IMAGE_DESCRIPTOR,
"Image creation must fail with CL_INVALID_IMAGE_DESCRIPTOR "
"when all are passed as NULL");

image.reset();

// Passing NULL properties and a valid image_format and image_desc
image =
clCreateImageWithProperties(context, NULL, CL_MEM_READ_WRITE,
&img_format, &image_desc, NULL, &errNum);
test_error(errNum,
"Unable to create image with NULL properties "
"with valid image format and image desc");

image.reset();

// Passing image_format as NULL
image = clCreateImageWithProperties(context, extMemProperties.data(),
CL_MEM_READ_WRITE, NULL, &image_desc,
NULL, &errNum);
test_failure_error(errNum, CL_INVALID_IMAGE_FORMAT_DESCRIPTOR,
"Image creation must fail with "
"CL_INVALID_IMAGE_FORMAT_DESCRIPTOR"
"when image desc passed as NULL");

image.reset();

// Passing image_desc as NULL
image = clCreateImageWithProperties(context, extMemProperties.data(),
CL_MEM_READ_WRITE, &img_format, NULL,
NULL, &errNum);
test_failure_error(errNum, CL_INVALID_IMAGE_DESCRIPTOR,
"Image creation must fail with "
"CL_INVALID_IMAGE_DESCRIPTOR "
"when image desc passed as NULL");
image.reset();

if (cmd_queue) clReleaseCommandQueue(cmd_queue);
if (context) clReleaseContext(context);

Expand Down
41 changes: 0 additions & 41 deletions test_conformance/vulkan/test_vulkan_api_consistency_for_3dimages.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,47 +162,6 @@ int test_consistency_external_for_3dimage(cl_device_id deviceID,
test_error(errNum, "Unable to create Image with Properties");
image.reset();

// Passing properties, image_desc and image_format all as NULL
image = clCreateImageWithProperties(context, NULL, CL_MEM_READ_WRITE, NULL,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case is ambiguous. both format and image desc are null, either case is an error, and the OpenCL spec does specify if CL_INVALID_IMAGE_FORMAT_DESCRIPTOR or CL_INVALID_IMAGE_DESCRIPTOR is to be returned if both are NULL.

This test case does not make sense for this extension, there is no information being passed that assosciates clCreateImageWithProperties and cl_khr_external_memory.

If we believe this is important coverage, it would be a better fit in api or basic.

NULL, NULL, &errNum);
test_failure_error(
errNum, CL_INVALID_IMAGE_DESCRIPTOR,
"Image creation must fail with CL_INVALID_IMAGE_DESCRIPTOR "
"when all are passed as NULL");

image.reset();

// Passing NULL properties and a valid image_format and image_desc
image =
clCreateImageWithProperties(context, NULL, CL_MEM_READ_WRITE,
&img_format, &image_desc, NULL, &errNum);
test_error(errNum,
"Unable to create image with NULL properties "
"with valid image format and image desc");

image.reset();

// Passing image_format as NULL
image = clCreateImageWithProperties(context, extMemProperties.data(),
CL_MEM_READ_WRITE, NULL, &image_desc,
NULL, &errNum);
test_failure_error(errNum, CL_INVALID_IMAGE_FORMAT_DESCRIPTOR,
"Image creation must fail with "
"CL_INVALID_IMAGE_FORMAT_DESCRIPTOR"
"when image desc passed as NULL");

image.reset();

// Passing image_desc as NULL
image = clCreateImageWithProperties(context, extMemProperties.data(),
CL_MEM_READ_WRITE, &img_format, NULL,
NULL, &errNum);
test_failure_error(errNum, CL_INVALID_IMAGE_DESCRIPTOR,
"Image creation must fail with "
"CL_INVALID_IMAGE_DESCRIPTOR "
"when image desc passed as NULL");
image.reset();

if (cmd_queue) clReleaseCommandQueue(cmd_queue);
if (context) clReleaseContext(context);

Expand Down
Loading