Add negative tests for context API functions #2494
Open
+297
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MichaelRizkalla-arm
changed the title
MichaelRizkalla-arm
force-pushed
the
negative_ctx_and_device_tests
branch
from
9b2681e to
2147c7e
Compare
MichaelRizkalla-arm
force-pushed
the
negative_ctx_and_device_tests
branch
from
2147c7e to
d99404d
Compare
EwanC
reviewed
Oct 28, 2025
Contributor
EwanC
left a comment
EwanC
left a comment
There was a problem hiding this comment.
Looking good, but have a few minor comments
| TEST_FAIL); | ||
|
|
||
| cl_context_properties invalid_value{ -1 }; | ||
| props[0] = CL_CONTEXT_INTEROP_USER_SYNC; |
Contributor
There was a problem hiding this comment.
we could enter this control flow if get_device_cl_version(device) returns Version(1,1) but CL_CONTEXT_INTEROP_USER_SYNC isn't added to the spec until version 1.2
Contributor
Author
There was a problem hiding this comment.
I updated the branch condition to >= Version(1, 2).
| "not one of the supported values\"", | ||
| TEST_FAIL); | ||
|
|
||
| err = clGetContextInfo(context, CL_CONTEXT_REFERENCE_COUNT, 0, ¶m_value, |
Contributor
There was a problem hiding this comment.
using a non-zero value, e.g sizeof(cl_uint) - 1 would be a marginally stronger test.
| ctx = clCreateContext(props, 1, &device, nullptr, nullptr, &err); | ||
| test_object_failure_ret( | ||
| ctx, err, CL_INVALID_PROPERTY, | ||
| "clCreateContext should return CL_INVALID_PROPERTY when: \"the " |
Contributor
There was a problem hiding this comment.
Does this provide extra coverage that the first check for this case on line 41 doesn't?
Contributor
Author
There was a problem hiding this comment.
No, it's been removed.
This change adds negative tests to `cl_context`-related APIs: - clCreateContext - clCreateContextFromType - clRetainContext - clReleaseContext - clSetContextDestructorCallback It also adds a verification macro: `test_object_failure_ret`. It verifies that an expected error code is returned and the CL object is `NULL`. Signed-off-by: Michael Rizkalla <michael.rizkalla@arm.com>
MichaelRizkalla-arm
force-pushed
the
negative_ctx_and_device_tests
branch
from
d99404d to
536a8dd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds negative tests for the following API functions:
Also, define a new macro
test_object_failure_retto test for an expected error code and the returned object isNULLas a result of a failure.