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.

Sign up for GitHub

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

Jump to bottom

Add/reorganize unit tests for CClaimTrieCache/CClaimTrie #104

Closed
10 tasks
kaykurokawa opened this issue Mar 12, 2018 · 4 comments · Fixed by #316
Closed
10 tasks

Add/reorganize unit tests for CClaimTrieCache/CClaimTrie #104

kaykurokawa opened this issue Mar 12, 2018 · 4 comments · Fixed by #316
Labels
type: improvement Existing (or partially existing) functionality needs to be changed
Milestone
Summer

Comments

@kaykurokawa
Copy link

kaykurokawa commented Mar 12, 2018 •
edited by alyssaoc
Loading

We have decent coverage for integration testing the CClaimTrieCache together with CClaimTrie but we do not have any unit tests for testing them separately.

Especially for CClaimTrieCache, adding unit tests would help developers understand and debug the class. We'll have better confidence on the correctness of the code going forward.

Acceptance Criteria

Definition of Done

  • Tested against acceptance criteria
  • Tested against the assumptions of the user story
  • The project builds without errors
  • Unit tests are written and passing
  • Tests on devices/browsers listed in the issue have passed
  • QA performed & issues resolved
  • Refactoring completed
  • Any configuration or build changes documented
  • Documentation updated
  • Peer Code Review performed
@kaykurokawa kaykurokawa added type: improvement Existing (or partially existing) functionality needs to be changed good first issue and removed needs: triage labels Jun 13, 2018
@kaykurokawa
Copy link
Author

Some tests were added here: #105 but could use more

@BrannonKing
Copy link
Member

Waiting on #175 's proposed "separate PRs".

@alyssaoc alyssaoc added the needs: grooming Issue needs to be groomed before work can start label Sep 13, 2018
@alyssaoc alyssaoc removed good first issue needs: grooming Issue needs to be groomed before work can start labels Sep 23, 2018
@BrannonKing
Copy link
Member

Having combined the two claimtrie unit test files as part of my other story to reuse unit test framework, I now agree with @lbrynaut 's comment about that being a bad move; I should have moved the unit test framework into its own file instead. The one unit test file is just too large (now knocking on 4k lines); it needs to be split into a number of smaller files.

@BrannonKing BrannonKing changed the title Add unit tests for CClaimTrieCache/CClaimTrie Add/reorganize unit tests for CClaimTrieCache/CClaimTrie Oct 24, 2018
@BrannonKing BrannonKing added this to the Summer milestone Mar 8, 2019
@bvbfan
Copy link
Collaborator

bvbfan commented Sep 11, 2019

#316

@bvbfan bvbfan mentioned this issue Sep 11, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: improvement Existing (or partially existing) functionality needs to be changed
Projects
None yet
Milestone
Summer
Development

Successfully merging a pull request may close this issue.

Organize unit tests by type lbryio/lbrycrd
5 participants
@BrannonKing @kaykurokawa @bvbfan @alyssaoc @lbry-bot