Possible fix for #914 compactCells failing when passed all res 1 cells #919
Conversation
src/apps/testapps/testCompactCells.c
Outdated
| int64_t numUncompacted = numRes1; | ||
| t_assertSuccess(H3_EXPORT(compactCells)(cells1, out, numUncompacted)); | ||
|
|
||
| // TODO: check that output matches cells0 |
There was a problem hiding this comment.
If everything else is passing, shall we add this check that we recover the res 0 cells?
There was a problem hiding this comment.
Sure, I'll add that too
| TEST(allRes1_variousRanges) { | ||
| const int64_t numRes0 = 122; | ||
| const int64_t numRes1 = 842; | ||
| H3Index *cells0 = calloc(numRes0, sizeof(H3Index)); | ||
| H3Index *cells1 = calloc(numRes1, sizeof(H3Index)); | ||
| H3Index *out = calloc(numRes1, sizeof(H3Index)); | ||
|
|
||
| H3_EXPORT(getRes0Cells)(cells0); | ||
| t_assert(cells0[0] == 0x8001fffffffffff, | ||
| "got expected first res0 cell"); | ||
|
|
||
| t_assertSuccess( | ||
| H3_EXPORT(uncompactCells)(cells0, numRes0, cells1, numRes1, 1)); | ||
|
|
||
| // Test various (but not all possible combinations) ranges of res 1 | ||
| // cells | ||
| for (int64_t offset = 0; offset < numRes1; offset++) { | ||
| for (int64_t numUncompacted = numRes1 - offset; numUncompacted >= 0; | ||
| numUncompacted--) { | ||
| memset(out, 0, sizeof(H3Index) * numRes1); | ||
|
|
||
| t_assertSuccess(H3_EXPORT(compactCells)(&cells1[offset], out, | ||
| numUncompacted)); | ||
| } | ||
| } | ||
|
|
||
| free(cells0); | ||
| free(cells1); | ||
| free(out); | ||
| } |
There was a problem hiding this comment.
Actually, I'm not positive that we need this test. I had code like this previously just to try to generate a minimal example of a failure. I don't think it hurts to add this test, but I don't think there's any reason to think this provides much additional value beyond compacting all the res 1 cells. Happy to go either way with it, based on your judgement @isaacbrodsky .
There was a problem hiding this comment.
It's not that strong a test. More convincing might be enumerating all possible sets of res 1 cells, but it might be useful for just ensuring all of these cases are not inadvertently failing.
Incorporates #914