[ETCM-739] Refactor BlockFetcher

[AnastasiiaL]

Change BlockFetcher to typed actor

Split fetcher message handling among child actors

Abstract the fetch trait

Scalafmt

Refactor blockFetcherState

[leo-bogastry]

What about tests for the new added classes?

[AnastasiiaL]

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

[leo-bogastry]

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

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.
I think individual tests (and more refined tests) on each of these (headers, bodies and states) are a good idea

[AnastasiiaL]

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

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.
I think individual tests (and more refined tests) on each of these (headers, bodies and states) are a good idea

let's ask @dzajkowski if we want to invest more time in this. AFAIK that wasn't our intention

[dzajkowski]

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

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.
I think individual tests (and more refined tests) on each of these (headers, bodies and states) are a good idea

let's ask @dzajkowski if we want to invest more time in this. AFAIK that wasn't our intention

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?

[AnastasiiaL]

follow-up on improving the tests https://jira.iohk.io/browse/ETCM-830

[dzajkowski]

quite o few watch statements

maybe it would make sense to add:

      .receiveSignal {
        case (context, Terminated(ref)) =>