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

keystore: Property based testing #3538

Closed
wants to merge 4 commits into from

Conversation

mateon1
Copy link
Contributor

@mateon1 mateon1 commented Dec 26, 2016

I'm playing around with property based testing, this PR implements a test that randomly performs Get(key), Put(key, value), Delete(key) and List() operations, and compares results from the keystore and its internal model. (a map of strings to keys)
On my Windows machine the test takes 17s to run, since gopter performs the test 100 times by default.

License: MIT
Signed-off-by: Mateusz Naściszewski <matin1111@wp.pl>
License: MIT
Signed-off-by: Mateusz Naściszewski <matin1111@wp.pl>
License: MIT
Signed-off-by: Mateusz Naściszewski <matin1111@wp.pl>
License: MIT
Signed-off-by: Mateusz Naściszewski <matin1111@wp.pl>
@whyrusleeping
Copy link
Member

Hrm... This is pretty nice to have. I wonder if we should have this as part of the main go-ipfs codebase, or a separate 'testing' repo that can have must larger and longer tests.

I also think we need to take a look at the library in use here to make sure its the one we want to move forward with.

cc @lgierth @Kubuxu @hsanjuan

@mateon1
Copy link
Contributor Author

mateon1 commented Jan 8, 2017

Regarding using Gopter for property tests, there exists a builtin alternative (testing/quick), but it doesn't support some common features for this kind of library, like shrinking (AKA finding the smallest failing testcase). The Gopter README explains it better: https://github.com/leanovate/gopter/#synopsis

@whyrusleeping
Copy link
Member

Hey @mateon1 thanks for this :) However i'm not sure we want to add 17 seconds (definitely more on slow CI boxes) to the tests, plus i'm always leery of adding new dependencies.

That said, gopter seems pretty cool. Any insight on how it compares to https://github.com/dvyukov/go-fuzz ?

@mateon1
Copy link
Contributor Author

mateon1 commented Apr 16, 2017

I somehow lost @whyrusleeping 's reply in my notifications, sorry.

There is a difference between fuzzing, and property testing.

In this PR, I use a pattern I first saw in Erlang automated testing, where you QuickCheck a series of commands, along with a simple state machine of what you expect to happen if these commands run. If there is a mismatch, that might be a potential bug.
Property testing can also be used to find multiple edge cases for simple functions using a single property, where you would normally write several tests (and possibly miss tricky cases! nil, empty string, zero, small integers, negative integers, integers near overflow, float Infinity/NaN, empty array, ..., duplicate calls for Add/Delete for the same key, ...).
Fuzzing is more appropriate (I feel) to file parsers/network handling code, where the fuzzer can intelligently find edge cases that do bad things in the (bigger) program.
While some of the bugs QC and fuzzing find may be the same, they are ultimately different methods.

I don't mind if this is closed and forgotten, as it was mostly just a go at learning gopter (and learning Go).

By the way, I'm probably going to have some fun with the go-fuzz thing as well :)

@Kubuxu
Copy link
Member

Kubuxu commented Apr 16, 2017

@mateon1 See my usage of go-fuzz for example in go-cid

@Kubuxu
Copy link
Member

Kubuxu commented Apr 20, 2017

@mateon1 I am trying to clean up PRs so I will close this.

@Kubuxu Kubuxu closed this Apr 20, 2017
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.

3 participants