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.

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

UT for createSubBuffer #254

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

piotr-wozniak-mobica
Copy link
Contributor

No description provided.

Comment on lines 4540 to 4547
static cl::Buffer ret;

clCreateSubBuffer_StubWithCallback(clCreateSubBuffer_testCreateSubBuffer);
ret = bufferPool[0].createSubBuffer(flags, buffer_create_type,
buffer_create_info, err);

TEST_ASSERT_EQUAL_PTR(make_mem(1), ret());
ret() = nullptr;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make this variable non-static:

Suggested change
static cl::Buffer ret;
clCreateSubBuffer_StubWithCallback(clCreateSubBuffer_testCreateSubBuffer);
ret = bufferPool[0].createSubBuffer(flags, buffer_create_type,
buffer_create_info, err);
TEST_ASSERT_EQUAL_PTR(make_mem(1), ret());
ret() = nullptr;
clCreateSubBuffer_StubWithCallback(clCreateSubBuffer_testCreateSubBuffer);
clReleaseMemObject_ExpectAndReturn(make_mem(1), CL_SUCCESS);
cl::Buffer ret = bufferPool[0].createSubBuffer(flags, buffer_create_type,
buffer_create_info, err);
TEST_ASSERT_EQUAL_PTR(make_mem(1), ret());

Comment on lines 4525 to 4528
TEST_ASSERT_EQUAL(make_mem(0), buffer);
TEST_ASSERT_EQUAL(0, flags);
TEST_ASSERT_EQUAL(0, buffer_create_type);
TEST_ASSERT_EQUAL_PTR(nullptr, buffer_create_info);
Copy link
Contributor

Choose a reason for hiding this comment

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

We really ought to be testing more real-world values here:

  • buffer - this one is OK.
  • flags - a value of 0 is legal, but it's not a very useful value to test to see if the flags are being passed through properly. Maybe pass something like CL_MEM_READ_ONLY instead?
  • buffer_create_type - this should be CL_BUFFER_CREATE_TYPE_REGION, since this is the only buffer creation type described in the spec.
  • buffer_create_info - this should be a non-NULL pointer to a cl_buffer_region structure. Ideally, we should check that the pointer is non-NULL, and that the contents of the structure are what we expect.

@kamil-goras-mobica
Copy link
Contributor

@bashbaug Added corrections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants