h3 icon indicating copy to clipboard operation
h3 copied to clipboard

Add test driver for h3-pg#56

Open isaacbrodsky opened this issue 3 years ago • 5 comments

Adds a test driver for bytesandbrains/h3-pg#56 to the core library for debugging this type of polygon that currently has a very high estimation.

isaacbrodsky avatar Jul 27 '21 22:07 isaacbrodsky

Coverage Status

Coverage remained the same at 98.993% when pulling 0953747e1e829fbf43bdc7c1597ebfa64c16902f on isaacbrodsky:h3pg-56-test into 4b574f04829ea7ca0edb73048ac4ef4833301f05 on uber:master.

coveralls avatar Jul 27 '21 22:07 coveralls

Not sure I'm following the goal of this PR - the test behavior seems correct. Is the idea that we'll have some strategy to identify cases like this in the future and allocate less memory?

nrabinowitz avatar Jul 28 '21 13:07 nrabinowitz

Not sure I'm following the goal of this PR - the test behavior seems correct. Is the idea that we'll have some strategy to identify cases like this in the future and allocate less memory?

Yes, this is to help test those cases in the C library, and also ensure we don't regress to requesting more memory in cases like this.

Aside: We may want to consider partial polyfill approaches that would allow us to provide a stream (or iterator) of results to the caller rather than requiring pre-allocating all the memory.

isaacbrodsky avatar Jul 29 '21 06:07 isaacbrodsky

Note also that the Win32 test for this actually fails with a segfault, so this does currently fail in some configurations.

isaacbrodsky avatar Jul 29 '21 07:07 isaacbrodsky

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Isaac Brodsky seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Feb 16 '23 07:02 CLAassistant