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

chore(lib/trie): refactor database tests #2338

Merged
merged 1 commit into from
Apr 27, 2022

Conversation

qdm12
Copy link
Contributor

@qdm12 qdm12 commented Mar 1, 2022

Changes

  • run in parallel
  • run with subtests (older)
  • extensible test case struct fields (older)
  • Store repeating values in functions (older)
  • Bigger test sets (can't use fuzzing)
  • Removed hardcoded test sets
  • Removed duplicate tests
  • Rename res tries to trieFromDB
  • Remove unneeded test code for loops
  • Clarify a few things here and there

ℹ️ New changes were made to have bigger random test sets since this helped me out debunk some problems in another branch

Tests

go test -race ./lib/trie

Issues

Primary Reviewer

@qdm12 qdm12 force-pushed the qdm12-trie-database-tests branch from 7606da9 to 4e9997d Compare March 1, 2022 17:03
@qdm12 qdm12 changed the base branch from development to qdm12-trie-branch-descendants March 1, 2022 17:04
@qdm12 qdm12 force-pushed the qdm12-trie-branch-descendants branch from 9e81d43 to 689751f Compare March 1, 2022 17:04
@qdm12 qdm12 force-pushed the qdm12-trie-database-tests branch 3 times, most recently from f08d09d to d43e9af Compare March 1, 2022 19:32
Copy link
Contributor

@noot noot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice copypasta removal

@qdm12 qdm12 marked this pull request as draft March 3, 2022 19:02
@qdm12 qdm12 force-pushed the qdm12-trie-branch-descendants branch from f33420d to c3dc9ab Compare March 7, 2022 16:15
@qdm12 qdm12 force-pushed the qdm12-trie-database-tests branch from d43e9af to ae15202 Compare March 7, 2022 17:04
@codecov
Copy link

codecov bot commented Mar 7, 2022

Codecov Report

Merging #2338 (613dffc) into development (3e95215) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           development    #2338      +/-   ##
===============================================
- Coverage        61.52%   61.52%   -0.01%     
===============================================
  Files              215      215              
  Lines            28267    28267              
===============================================
- Hits             17391    17390       -1     
+ Misses            9105     9104       -1     
- Partials          1771     1773       +2     
Flag Coverage Δ
unit-tests 61.52% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/trie/database.go 49.39% <0.00%> (-2.75%) ⬇️
lib/runtime/wasmer/imports.go 48.25% <0.00%> (-0.20%) ⬇️
lib/blocktree/blocktree.go 54.71% <0.00%> (+1.08%) ⬆️
dot/state/block_notify.go 90.47% <0.00%> (+8.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f521c97...613dffc. Read the comment docs.

@qdm12 qdm12 force-pushed the qdm12-trie-branch-descendants branch from 39ab29e to 24bb458 Compare March 9, 2022 14:12
@qdm12 qdm12 force-pushed the qdm12-trie-database-tests branch from ae15202 to 3eebd91 Compare March 9, 2022 14:19
Base automatically changed from qdm12-trie-branch-descendants to qdm12-trie-copy-less March 14, 2022 19:10
@qdm12 qdm12 force-pushed the qdm12-trie-copy-less branch 2 times, most recently from 80998f2 to 917e1c2 Compare March 15, 2022 16:01
@qdm12 qdm12 changed the base branch from qdm12-trie-copy-less to qdm12-trie-branch-descendants March 15, 2022 16:08
@qdm12 qdm12 force-pushed the qdm12-trie-database-tests branch from 3eebd91 to 0091757 Compare March 15, 2022 16:09
@qdm12 qdm12 force-pushed the qdm12-trie-branch-descendants branch 2 times, most recently from e0eee15 to ba3c5dd Compare March 16, 2022 16:05
@qdm12 qdm12 force-pushed the qdm12-trie-database-tests branch from 0091757 to bafc15f Compare March 16, 2022 16:18
@qdm12 qdm12 force-pushed the qdm12-trie-branch-descendants branch from ba3c5dd to 908596a Compare March 23, 2022 10:25
@qdm12 qdm12 force-pushed the qdm12-trie-branch-descendants branch from 908596a to 4b32aaf Compare April 26, 2022 08:39
@qdm12 qdm12 force-pushed the qdm12-trie-database-tests branch from bafc15f to 19d419b Compare April 26, 2022 13:02
@qdm12 qdm12 changed the base branch from qdm12-trie-branch-descendants to development April 26, 2022 13:02
@qdm12
Copy link
Contributor Author

qdm12 commented Apr 26, 2022

I changed the refactor further to use randomly generated key values. This helped me out find a bug in another branch, so it definitely has a use. Unfortunately, I don't think fuzzing can be applied in this particular case.

@qdm12 qdm12 marked this pull request as ready for review April 26, 2022 15:14
@qdm12 qdm12 requested a review from noot April 27, 2022 12:28
Copy link
Member

@EclesioMeloJunior EclesioMeloJunior left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvements!! LGTM

- Bigger test sets (can't use fuzzing)
- Removed hardcoded test sets
- Removed duplicate tests
- Rename `res` tries to `trieFromDB`
- Remove unneeded test code for loops
@qdm12 qdm12 force-pushed the qdm12-trie-database-tests branch from 19d419b to 613dffc Compare April 27, 2022 14:14
@qdm12 qdm12 merged commit 91e7ffa into development Apr 27, 2022
@qdm12 qdm12 deleted the qdm12-trie-database-tests branch April 27, 2022 16:28
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