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

Write tests for gc_realloc() #44

Closed
mkirchner opened this issue Jan 8, 2020 · 5 comments · Fixed by #51
Closed

Write tests for gc_realloc() #44

mkirchner opened this issue Jan 8, 2020 · 5 comments · Fixed by #51
Labels
feature New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@mkirchner
Copy link
Owner

We don't have any tests for gc_realloc() in test_gc.c yet, those would be great to have.

@mkirchner mkirchner added feature New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Jan 8, 2020
@ganesh-k13
Copy link
Contributor

So the checks/asserts that need to be done are:

  1. Pass an invalid pointer to realloc and check for NULL return.
  2. Pass NULL ptr and check for alloc.
  3. Do a normal realloc.

In all the cases we do a gc_allocation_map_get and assert the size.

Is this it or anything else to cover? I'll wait a few days for any new contributors else, I'll take it up.

@mkirchner
Copy link
Owner Author

@ssrlive Deleting your comment here, moved it to #47 .

Repository owner deleted a comment from ssrlive Jan 14, 2020
@mkirchner
Copy link
Owner Author

@ganesh-k13 yes, that makes sense. With gc_realloc() it's important to test every code path (the realloc() fail is ok to ignore, not worth the effort here) and a combination of your suggestions and confirming it on coveralls should give you all required cases.

@ganesh-k13
Copy link
Contributor

Hey @mkirchner, this line:

gc/src/gc.c

Line 448 in e5ebe36

gc_allocation_map_put(gc->allocs, p, size, alloc->dtor);

it should be q right, not p?

@mkirchner
Copy link
Owner Author

Nicely spotted! ... and that’s why we need tests for gc_realloc() ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants