[ETCM-739] Refactor BlockFetcher#976
Conversation
AnastasiiaL
force-pushed
the
refactor-fetcher-step-by-step
branch
from
9876ed2 to
70a77ee
Compare
|
What about tests for the new added classes? |
there is no new functionality added, it was just split for readability. Existing BlockFetcher tests still cover those |
AnastasiiaL
force-pushed
the
refactor-fetcher-step-by-step
branch
from
70a77ee to
791117e
Compare
I think my issue is that when I see a class being split I also expect tests to be split. The BlockFetcherSpec has only 7 scenarios, involving fetching all headers, bodies and state. |
let's ask @dzajkowski if we want to invest more time in this. AFAIK that wasn't our intention |
AnastasiiaL
force-pushed
the
refactor-fetcher-step-by-step
branch
from
791117e to
0db4b72
Compare
summoning circle failed, please try again in 12hrs summoning circle succeeded I agree with @LeonorLunatech that tidying up the tests and having a more focused set of specs makes sense but I also am aware that this refactor did not bring new functionalities to the table - so we would be making sure that akka typed works better. I'm fine with having such an improvement as a follow up ticket. please put it in 'tech debt' and mark as 'good first issue', @AnastasiiaL. @LeonorLunatech is that an acceptable compromise? |
|
follow-up on improving the tests https://jira.iohk.io/browse/ETCM-830 |
AnastasiiaL
force-pushed
the
refactor-fetcher-step-by-step
branch
from
0db4b72 to
5cba7bf
Compare
Change BlockFetcher to typed actor Split fetcher message handling among child actors Abstract the fetch trait Scalafmt Refactor blockFetcherState Fix it tests Fix scalastyle, comments Fix comments Add logging
AnastasiiaL
force-pushed
the
refactor-fetcher-step-by-step
branch
from
5cba7bf to
61503fa
Compare
| "block-fetcher" | ||
| ) | ||
|
|
||
| context.watch(fetcher) |
There was a problem hiding this comment.
quite o few watch statements
maybe it would make sense to add:
.receiveSignal {
case (context, Terminated(ref)) =>
3 participants
Change BlockFetcher to typed actor
Split fetcher message handling among child actors
Abstract the fetch trait
Scalafmt
Refactor blockFetcherState