test: add leaf_stage property tests

[gabriele-0201]

No description provided.

[gabriele-0201]

• test: add leaf_stage property tests #493 👈
• fix: stop split from creating too big right leaf #492
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @gabriele-0201 and the rest of your teammates on Graphite

[rphmeier]

I like the general direction, but there are some issues before I'm comfortable merging this.

It's structurally a code smell for a test of the beatree to bring in NOMT, file I/O, and so on. A test for a particular component shouldn't need to bring in the entire application.

They also look extremely heavy, i.e. there is no way we will be able to run these in CI, and so it looks unlikely that the tests will ever be run.

A couple of suggestions:

1. Find a way to do these tests in-memory to avoid the load and confounding factors of disk I/O. Perhaps using tmpfile or memfd (less portable) to create the FDs for the BBN/LN files.
2. Find a way to test the desired properties without such large databases.
3. Find a way to isolate just beatree functionality in these tests. Prefer not to initialize a full NOMT.

[rphmeier]

As a follow-on to my last comment.

I just checked out the PR and ran the tests. They are using 100% of my CPU and have been running for over 2 minutes.

[gabriele-0201]

Interesting, I developed them under some logical flaw. What I was thinking is that property testing is similar to fuzzing but with a narrower scope. Thus, I was thinking that the tester should be able to "experiment" with different inputs iteratively until a problem is found. That is also why I am initializing a proper database on which the leaf stage is tested.

It seems to me that what you are requiring as property testing is something closer to unit tests than fuzzing – just a unit test with random input with the shrinking feature.

I think I understood your point, and thank you for the suggestions. What I will try to do now, logically, is initialize just the beatree all in memory (perhaps using tmpfile or memfd) and run on top of it the same tests.

However, before even trying it, I'm skeptical about the feasibility of initializing the beatree without any import from other modules rather than the beatree module. But if that's impossible, I will find some workaround!

[gabriele-0201]

notes after edits:

• the test focused on range extension is being removed to make tests faster
• the number of initial values inside the database has been reduced
• on average, 25k leaves are built during the leaf_stage test, and around 3k range request extensions are issued
• ln_fd and bbn_fd are now backed by tmpfile. memfd would probably have been faster, but it would not work on macOS
• now it is faster than before and does not use 100% of the CPU
• io uring with iopoll doesn't work on tmpfiles so a flag has been added to build or not io_uring with iopoll

[deuszx]

You could also try reading an env first (like NOMT_PRNG_TEST_SEED) and in the custom Drop impl for your wrapper, when a test fails (if thread::panicking()) log the PRNG. This way you can reproduce the tests locally on your machine when they fail in CI (or on someone else's PC).

[rphmeier]

That looks like a good suggestion. want to make a follow-up PR?

[gabriele-0201]

That's a super nice idea, it would be very helpful ! Let us know if you want to work on that!

[deuszx]

Sure, I can try. Give me time until tomorrow.

[gabriele-0201]

Hello! It's been almost a month, and sadly, we really need to have reproducible bugs in CI. Thus, I had to implement something very similar to what you suggested! #570

I didn't use the environment variable because I personally think it's easier to copy and paste the seed into the code. However, if you think it's valuable, there is still room for that update!

I tried to use if thread::panicking() inside a drop, but I discovered that drop is not performed on static elements (which makes sense). Thus, given how quickcheck lets you build and execute the tests, there were no other non-static struct which could easily implement the Drop trait.

Given those constraints, I went for the try and catch approach, which simply prints the seed if a panic occurs during the tests, and then it resumes the unwinding process.

[rphmeier]

Looks good. Is it possible to add the extension protocol test back in the future?

Perhaps we can use property tests in a more focused way to test that.

[gabriele-0201]

Is it possible to add the extension protocol test back in the future?

The same test that was there before does not work well with small dbs

Perhaps we can use property tests in a more focused way to test that

Already, two properties are ensured to be respected: there should be no underfull and all preloaded leaves should be deleted. The test ensure that both the update procedure and the extension protocol maintain these properties. One more thing that can be done is testing for data loss. It should be ensured that no existing value is lost during an update or a range extension