Re: dshash_find_or_insert vs. OOM

> You need to get rid of the bare blocks.

done in v4

> This verifies that dshash_dump() doesn't crash, but not that it
> produces useful output. Whether that's worth the infrastructure, I
> don't know.

I don't think dshash_dump() will be deterministic, as keys may hash
to different buckets on different platforms, perhaps. Which is why
I did not think it was a good idea to compare the output from the logs.
Testing that it does not crash seemed worthwhile to increase
 the coverage.

> 408). The omissions are: some OOM cases, dshash_delete_bucket()
> successfully deletes something, concurrency case in resizing, most of
> delete_key_from_bucket(), delete_item_from_bucket() iterating more

Yes, we can probably address some of these things with more complexity.

> than once or returning false. Combining your patch with the test suite
> previously mentioned, we get up to 97.1% coverage by lines (396 of
> 408).

This is solid coverage

> So the patch definitely has some value: it adds coverage of 60+ lines
> of code. On the other hand, it still feels to me like we'd be far more
> likely to notice new dshash bugs based on its existing usages than
> because of this. If I were modifying dshash, I wouldn't run this test
> suite first; I'd run the regression tests first.

Sure, but once a bug is fixed, having direct test coverage fo this bug
in a dedicated test suite is a good thing.

The NO_OOM flag that started this discussion would not fit neatly in other
test suites, AFAICT.

--
Sami