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

Testdata statistics #31

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

Testdata statistics #31

wants to merge 12 commits into from

Conversation

alfert
Copy link

@alfert alfert commented Nov 10, 2021

This PR implements the proposal of #30.

I am using interface rapid.TB instead of rapid.T to introduce a test adapter for checking the PrintStats() methods. That reads a little odd, any improvement here are welcome.

@flyingmutant
Copy link
Owner

On the implementation side, event tracking should be thread safe. Does using test name as an ID work with go test -count=N?

I'd prefer to use *rapid.T, as I think clarity of public API is more important than internal testing convenience. Also, maybe PrintStats should be internal and be called automatically if any events were emitted (and a command-line flag was set, maybe)?

Continuing that thought, Hypothesis does a good job showing summary of data generation even when no custom events were emitted. Do you think having "just" custom events is useful enough, or maybe we should try to start with built-in event/summary thing first, and add custom events on top (like Hypothesis does)? That way, regular users get useful functionality with no additional APIs to use (simply add a new command-line flag and gain insight into the data generation).

@alfert
Copy link
Author

alfert commented Nov 12, 2021

@flyingmutant very good points, thread safety was not on my checkmark. That happens if you are for a long time in the Erlang/Elixir world :-)

I really like the idea to call PrintStats automatically, since it will be forgotten most of the time, I am pretty sure.

The internal events from Hypothesis can be improved, simply due to not reaching 100% or show even more. This is confusing. But it could be healed by enabling several reports for one test case. Things like filter would open up a separate category (which should sum up to 100% during reports). This is inspired by Erlang QuickCheck (http://quviq.com/documentation/eqc/index.html, function collect/3) and PropEr (http://propertesting.com/book_custom_generators.html#_collect). With several categories, we can distinguish between different generators in a property, which is also handy.

I'll try to find some time during the weekend to rework the PR.

@flyingmutant
Copy link
Owner

Great, looking forward to then next version!

@flyingmutant flyingmutant linked an issue Nov 12, 2021 that may be closed by this pull request
@alfert
Copy link
Author

alfert commented Nov 17, 2021

@flyingmutant Next round, but there is more to be done. What I did:

  • statistics are printed automatically for a property, if events are send
  • event collection is done asynchronously with go-routines and channels
  • API changed to allow for several independent collections within a testcase
  • Tests are adapted to new API and do no longer depend on a specific T-variant.

What is still missing before done:

  • Automatic events introduced by filters or maps
  • nicer formatted output
  • documentation enhancements

Open question (advice required):

  • how to do stats output? I tried to use t.Logf() but this did not work even if I use -v as test run parameter. Did I miss something? Is -rapid.v required as test flag? The log.Printf() is of course only an interim solution, which is quite visibly not the final solution.

Possible extension

  • Add a NumericEvent function, which puts values in a different collection. This gives the opportunity to calculate standard measures like mean, median, deviation and the like.

event.go Outdated Show resolved Hide resolved
 It uses now a mutex to protect both,
 writing to the stats and printing the stats.
The channels are gone. Since printing is done after finishing the property,
there should be most likely no still running
go-routines exposing event - but if so,
then we will miss them. This is ok.
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.

Computing statistics on generated test values
2 participants