Reorganised urEventGetProfilingInfo test into separate tests instead of a switch #2626
Conversation
martygrant
force-pushed
the
martin/getinfo-property-rename
branch
4 times, most recently
from
b7a32e5 to
fb30178
Compare
| ASSERT_SUCCESS(urKernelGetInfo(kernel, property_name, property_size, | ||
| property_value.data(), nullptr)); | ||
|
|
||
| ASSERT_EQ(property_size, property_value.size()); |
There was a problem hiding this comment.
NIT: Isn't this just testing that the standard library's std::vector class works rather than UR?
There was a problem hiding this comment.
so my intention was to test that the entry point populates pPropValue and that its size matches the size retrieved from the first call
There was a problem hiding this comment.
Are you expecting the std::vector's size to be changed by the call to urKernelGetInfo()? If so that's never going to happen, only the data owned by the std::vector will change not the container itself.
There was a problem hiding this comment.
discussed this offline and cleared up my misunderstanding around testing for this, will update the PR with a better check
There was a problem hiding this comment.
I've removed these size checks and added a helper function uur::stringPropertyIsValid to validate char[] returns
…of a switch and rework various urXGetInfo tests: - Housekeeping pass to tidy up tests, renaming and adding consts where appropriate - Added ASSERT_INFO_CALL and uur::stringPropertyIsValid helpers from oneapi-src#2597 - Used above helpers across all tests to better validate numerical and char[] queries, and reworked some char[] queries - Removed any non useful checks on vector sizes after passing them into a urXGetInfo query
martygrant
force-pushed
the
martin/getinfo-property-rename
branch
from
fb30178 to
f2be414
Compare
|
I've added some more changes to this PR, tidying up some more |
For #2290
Also done a housekeeping pass across all urXGetInfo tests, renaming and adding consts where appropriate, and adding extra checks to tests with vectors.