Skip to content

Key tree property tests#1144

Merged
nickva merged 1 commit into
masterfrom
property-tests-for-couch-key-tree
Apr 3, 2018
Merged

Key tree property tests#1144
nickva merged 1 commit into
masterfrom
property-tests-for-couch-key-tree

Conversation

@nickva
Copy link
Copy Markdown
Contributor

@nickva nickva commented Feb 6, 2018

Key tree module is a candidate to use property tests on as it mostly deals with
manipulating a single data structure and functions are referentially
transparent, that is, they aren't many side-effects like IO for example.

The test consists of two main parts - generators and properties.

Generators generate random input, for example revision trees, and properties
check that certain properties hold, for example that after stemming all the
leaves are still present in the revtree.

To run the test:

make eunit apps=couch suites=couch_key_tree_prop_tests

  • Code is written and works correctly;
  • Changes are covered by tests;
  • Documentation reflects the changes;

@nickva nickva force-pushed the property-tests-for-couch-key-tree branch from 09084af to a8f4009 Compare February 6, 2018 16:48
@nickva nickva mentioned this pull request Feb 6, 2018
3 tasks
@wohali wohali requested review from davisp and janl March 26, 2018 23:31
@wohali
Copy link
Copy Markdown
Member

wohali commented Mar 26, 2018

Need a review from someone more familiar with this code than I, I'm not willing to +1 this on my own.

Also needs the conflict resolved in rebar.config.script.

@nickva nickva force-pushed the property-tests-for-couch-key-tree branch 2 times, most recently from dad17cd to 24cd087 Compare April 3, 2018 18:37
@eiri
Copy link
Copy Markdown
Member

eiri commented Apr 3, 2018

I am willing to +1 it, the tests themselves look good, but they, understandably, take a long time to run. I just re-run just that one suite on my not too weak MBP and it took [done in 287.081 s], so I'm afraid it'll kick Travis and Jenkins in the guts if it'd be part of standard eunit run.

@nickva do you think we can move it to a separate make target somehow? It's nice to have those, don't get me wrong, but they are quite specific bit, so we probably don't need to run them for every change?

@nickva
Copy link
Copy Markdown
Contributor Author

nickva commented Apr 3, 2018

@eiri @wohali Thanks for taking a look

I rebased it. I'll see maybe I'll try to reduce the branchiness and the generated sizes a bit to speed things up. So maybe it adds another 2-3 minutes but not 5

How are these times looking?

module 'couch_key_tree_prop_tests'
  couch_key_tree_prop_tests:49: revtree_merge_random_nodes_test_...[0.493 s] ok
  couch_key_tree_prop_tests:154: after_stemming_paths_are_shorter_test_...[0.269 s] ok
  couch_key_tree_prop_tests:175: leaf_count_test_...[0.255 s] ok
  couch_key_tree_prop_tests:136: after_stemming_all_leaves_are_present_test_...[0.262 s] ok
  couch_key_tree_prop_tests:99: stemming_results_in_same_or_less_total_revs_test_...[0.260 s] ok
  couch_key_tree_prop_tests:63: revtree_merge_some_existing_some_new_test_...[23.715 s] ok
  couch_key_tree_prop_tests:117: stem_path_expect_size_to_get_smaller_test_...[5.431 s] ok
  couch_key_tree_prop_tests:182: get_leafs_test_...[0.224 s] ok
  couch_key_tree_prop_tests:32: revtree_merge_with_subset_of_own_nodes_test_...[35.110 s] ok
  couch_key_tree_prop_tests:84: no_change_stemming_deeper_than_current_depth_test_...[0.253 s] ok
  [done in 66.302 s]

Key tree module is a candidate to use property tests on as it mostly deals with
manipulating a single data structure and functions are referentially
transparent, that is, they aren't many side-effects like IO for example.

The test consists of two main parts - generators and properties.

Generators generate random input, for example revision trees, and properties
check that certain properties hold, for example that after stemming all the
leaves are still present in the revtree.

To run the test:

make eunit apps=couch suites=couch_key_tree_prop_tests
@nickva nickva force-pushed the property-tests-for-couch-key-tree branch from 24cd087 to 716ca59 Compare April 3, 2018 18:53
@eiri
Copy link
Copy Markdown
Member

eiri commented Apr 3, 2018

Ok, [done in 67.107 s] locally and ~30mins on Travis, I think we can live with that. Merge it and we'll see what Jenkins have to say.

@nickva nickva merged commit 02c9429 into master Apr 3, 2018
@nickva nickva deleted the property-tests-for-couch-key-tree branch April 3, 2018 20:48
@wohali
Copy link
Copy Markdown
Member

wohali commented Apr 9, 2018

For posterity, we need to mirror new dependencies into Apache infrastructure when adding them - not just check that they are compatibly licensed. This should have been done at the same time as this merge landing.

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