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

Full mock coverage #13

Merged
merged 20 commits into from
Nov 3, 2021
Merged

Full mock coverage #13

merged 20 commits into from
Nov 3, 2021

Conversation

weswalla
Copy link
Collaborator

@weswalla weswalla commented Nov 2, 2021

an hdk_crud refactor to be able to mock all the exposed functions for unit testing on the consumer side.

closes #12

  • convert all inner_ crud functions into generic actions
  • update comments to describe the generic actions
  • use mock feature
  • make the remaining hdk_crud functions mockable (turn into struct methods)

Follow ups (split into separate issue and PR)

@weswalla
Copy link
Collaborator Author

weswalla commented Nov 2, 2021

make remaining hdk_functions mockable

current plan:

  1. split remaining functions in retrieval.rs into their own file and make them mockable (like get_latest_for_entry.rs)
    image

  2. make generic crud action functions (in crud.rs) mockable.

  • question: each function will become a struct method such that it can be mocked. Should these structs be defined in crud.rs, or split into their own files like the other mocked functions?

@Connoropolous
Copy link
Collaborator

@WFinck97
I'm thinking...
along with 'retrieval' which already exists, there could be chain_actions mod...
in it would be a file for create_action update_action delete_action. Separated because these actions affect the source chain

Copy link
Collaborator

@Connoropolous Connoropolous left a comment

Choose a reason for hiding this comment

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

I think all my change requests are expressed as mergeable suggestions.
You can use the bulk 'add suggestion to batch' feature to merge them all at once.

So close! Excellent

src/chain_actions/fetch_action.rs Outdated Show resolved Hide resolved
Comment on lines +3 to +9
/// A triple of an Entry along with the HeaderHash
/// of that committed entry and the EntryHash of the entry
pub type EntryAndHash<T> = (T, HeaderHash, EntryHash);

/// The same as an EntryAndHash but inside an Option,
/// so it can be Some(...) or None
pub type OptionEntryAndHash<T> = Option<EntryAndHash<T>>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

would prefer these to be in a spot like 'types' or something, but not going to ask that to be done right now.

src/crud.rs Outdated Show resolved Hide resolved
src/crud.rs Outdated Show resolved Hide resolved
src/chain_actions/create_action.rs Outdated Show resolved Hide resolved
@weswalla weswalla mentioned this pull request Nov 3, 2021
weswalla and others added 2 commits November 3, 2021 16:28
@Connoropolous Connoropolous marked this pull request as ready for review November 3, 2021 23:35
@Connoropolous Connoropolous merged commit db20980 into main Nov 3, 2021
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.

automock functions not yet mockable
2 participants