light-client: turn Handle into a trait #401
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As we start to depend on the surface of the `Handle` we benefit from it being a trait that can be implemented on a per need basis. This will result in less overhead constructing object graphs in places where we wannt to assert behaviour of other types in remote places, i.e. the light-node rpc server. Overall we hope for an increased ease in writing tests on module level. Ref #219
xla
added
enhancement
This was referenced Jun 30, 2020
Merged
liamsi
reviewed
Jul 1, 2020
|
|
||
| /// Terminate the underlying [`Supervisor`]. | ||
| fn terminate(&mut self); | ||
| } |
There was a problem hiding this comment.
Looks good to me 👍 My understanding of why making this a trait is cool is because it makes testability easier. Is there a (wip) example of such a test that would be annoying to write without this (maybe in another PR)?
There was a problem hiding this comment.
The commit a452244 shows the start, where the test code implements a mockHandle. Generally it means a reduction of the objects that need constructing. If something was dependent on the Handle before, one had to provide a PeerList, ForkDetector, EvidenceProvider to construct the Supervisor. On all the state inside had to be correct in order to produce output as expected, which means the correct state in peer(s) stores, etc.
liamsi
reviewed
Jul 1, 2020
Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
brapse
reviewed
Jul 1, 2020
brapse
reviewed
Jul 1, 2020
|
Looks good :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As we start to depend on the surface of the
Handlewe benefit from it being a trait that can be implemented on a per need basis. This will result in less overhead constructing object graphs in places where we want to assert behaviour of other types in remote places, i.e. the light-node RPC server. Overall we hope for an increased ease in writing tests on module level.Ref #219
Ref #398
Updated all relevant documentation in docsWrote tests