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

Add property-based testing #81

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

lihaohong6
Copy link

Randomized testing seems to be planned in #63 but was never actually done. This is an attempt at it using the rapidcheck library.

3 crashes have been found so far. They are documented by FIXMEs. They all happen when malformed inputs are supplied to the library. For example, find_end segfaults if the first list is shorter than the second list due to a bad index access, though it is supposed to return list1.end().

Most functions tested by test_primitives.cpp have been covered. More tests (and some documentation on running these tests) are forthcoming.

@lihaohong6
Copy link
Author

@DanielLiamAnderson Any thoughts on this PR? I'll keep working on polishing the code/documentation if you think this patch can potentially be merged.

@DanielLiamAnderson
Copy link
Contributor

I'm not familiar with the RapidCheck library but this seems useful for sure.

@lihaohong6
Copy link
Author

I'm not familiar with the RapidCheck library but this seems useful for sure.

RapidCheck seems to be the only PBT/fuzzing library around for C++. It has not had any activities for 2 years: looks like the author has abandoned it. Fortunately, the library is feature-complete enough.

I got some time to work on this more. The README now has some explanations on running these newly-added tests. I also added some more tests so that everything covered by test_primitives.cpp is covered by test_primitives_property_based.cpp as well, though I might have missed a function here and there.

One more bug is found: in remove_duplicate_integers, if the input sequence is empty, it'll result in a SIGFPE because collect_reduce_few tries to divide something by block_size, which is 0. Another issue is that the documentation stated

r must be a range of integer type of value at most max_value

However, in practice, calling remove_duplicate_integers({0, 1}, 1) would crash the program. Changing max_value to 2 by calling remove_duplicate_integers({0, 1}, 2) would not result in any errors. To remain consistent with the programs behavior, should the docs say "less than" instead of "at most"?

Looks like not all CI tests passed last time. I'll investigate this more once I have time.

@lihaohong6
Copy link
Author

I switched from using addition to xor in some of the tests so that UBSAN wouldn't complain about integer overflow. A few parlay functions perform implicit type casts that change the signedness, so I changed them as well to make UBSAN happy.

As for DWARF error: invalid or unhandled FORM value: 0x23, I suspect that this issue is the culprit. Clang produces debugging symbols in DWARF5, which older versions of ld fail to parse in special circumstances. Incidentally, these errors only occur with clang++ being the compiler. I'm not sure how to fix it besides changing the CI workflow (e.g. use lld instead of ld or install newer versions of build tools from Ubuntu PPAs).

@lihaohong6
Copy link
Author

There's also a failed build due to the use of exceptions in rapidcheck. I'm not sure what is the best way to deal with it. Since most people will only need the headers in include, I would assume using exceptions in tests does not affect the goal of making the library exception-free.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants