Fix: unit tests in consensus, p2p and persistence modules

[Prajjawalk]

Description

This PR fixes all the unit test functions in all modules where actual and expected values were misplaced in require.Equal check.

Issue

Fixes #241

Type of change

Please mark the relevant option(s):

• New feature, functionality or library
• Bug fix
• Code health or cleanup
• Major breaking change
• Documentation
• Other

List of changes

• Changes in consensus module - Fix TestHotstuff4Nodes1BlockHappyPath misplacement of actual and expected values in require.Equal.
• Changes in p2p module - Updated TestRainTreeAddrBookUtilsHandleUpdate and testRainTreeMessageTargets to correct incorrect expected and actual value placements.
• Changes in persistence module - Fix unit tests - TestGetAppPauseHeightIfExists, TestGetAppOutputAddress, TestGetFishermanStatus, TestGetFishermanPauseHeightIfExists, TestGetFishermanOutputAddress, TestPersistenceContextParallelReadWrite, TestGetServiceNodePauseHeightIfExists, TestGetServiceNodeOutputAddress, fuzzSingleProtocolActor, TestGetValidatorPauseHeightIfExists, and TestGetValidatorOutputAddress for misplaced expected and actual values in require.Equal.
• Changes in utility module - Updated TestUtilityContext_SetPoolAmount, TestUtilityContext_GetMessageEditStakeSignerCandidates, TestUtilityContext_GetMessageUnpauseSignerCandidates, TestUtilityContext_GetMessageUnstakeSignerCandidates, and TestUtilityContext_UnstakePausedBefore to correct misplaced expected and actual values in require.Equal.

Testing

• make develop_test
• LocalNet w/ all of the steps outlined in the README

Required Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have tested my changes using the available tooling
• I have updated the corresponding CHANGELOG

If Applicable Checklist

• I have updated the corresponding README(s); local and/or global
• I have added tests that prove my fix is effective or that my feature works
• I have added, or updated, mermaid.js diagrams in the corresponding README(s)
• I have added, or updated, documentation and mermaid.js diagrams in shared/docs/* if I updated shared/*README(s)

[Olshansk]

@Prajjawalk The changes LGTM. Two things:

1. Is this an exhaustive update of all the tests? In other words, do you think others still need to be found or did you review all of them?

2. Please add a description to the GitHub PR - can be short.

[Prajjawalk]

@Prajjawalk The changes LGTM. Two things:

1. Is this an exhaustive update of all the tests? In other words, do you think others still need to be found or did you review all of them?
2. Please add a description to the GitHub PR - can be short.

Yes @Olshansk , I have reviewed unit tests of all modules.

[Olshansk]

@Prajjawalk The PR looks good to me, but our team (cc @okdas) is trying to figure out why the tests fail locally but pass in the CI.

1. I know you've tested the changes yourself, but could you confirm if this passes for you: go test -v -count=1 -run TestStateHash_DeterministicStateWhenUpdatingAppStake ./persistence/..?
2. Similarly, does make develop_test work for you?
3. At the moment, it seems that we need to make the following change to revert the broken test on mainline:

--- a/persistence/test/state_test.go
+++ b/persistence/test/state_test.go
@@ -48,9 +48,9 @@ func TestStateHash_DeterministicStateWhenUpdatingAppStake(t *testing.T) {
        // that the business logic doesn't change and that they remain deterministic. Anytime the business
        // logic changes, these hashes will need to be updated based on the test output.
        stateHashes := []string{
-               "d719c5e760529556095dc683b1d146f02743babdd11c6c9d5f630c18de031d8b",
-               "90b0554a8b87f823bb3ab91bf5bffe94deb7b05dafc4a7a6072ccd39c04a70e2",
-               "486dab2407ea4817fd1ad35113fbb52604c9c95e6155517c7466663f5de33f0e",
+               "855f415b61748de482e3a547a4c88dd68fce23ca604884bc37881246c07988c6",
+               "8b874d45c555770b2eb90f0b00b3dca1b9f98e3785cffa2d9e7642379cb07ae7",
+               "0ddfd05a6dcb205c8c946e44847071e583f8c6ddd0757bc55b11b5200c34856f",
        }

        stakeAmount := initialStakeAmount

[Prajjawalk]

@Prajjawalk The PR looks good to me, but our team (cc @okdas) is trying to figure out why the tests fail locally but pass in the CI.

1. I know you've tested the changes yourself, but could you confirm if this passes for you: go test -v -count=1 -run TestStateHash_DeterministicStateWhenUpdatingAppStake ./persistence/..?
2. Similarly, does make develop_test work for you?
3. At the moment, it seems that we need to make the following change to revert the broken test on mainline:

--- a/persistence/test/state_test.go
+++ b/persistence/test/state_test.go
@@ -48,9 +48,9 @@ func TestStateHash_DeterministicStateWhenUpdatingAppStake(t *testing.T) {
        // that the business logic doesn't change and that they remain deterministic. Anytime the business
        // logic changes, these hashes will need to be updated based on the test output.
        stateHashes := []string{
-               "d719c5e760529556095dc683b1d146f02743babdd11c6c9d5f630c18de031d8b",
-               "90b0554a8b87f823bb3ab91bf5bffe94deb7b05dafc4a7a6072ccd39c04a70e2",
-               "486dab2407ea4817fd1ad35113fbb52604c9c95e6155517c7466663f5de33f0e",
+               "855f415b61748de482e3a547a4c88dd68fce23ca604884bc37881246c07988c6",
+               "8b874d45c555770b2eb90f0b00b3dca1b9f98e3785cffa2d9e7642379cb07ae7",
+               "0ddfd05a6dcb205c8c946e44847071e583f8c6ddd0757bc55b11b5200c34856f",
        }

        stakeAmount := initialStakeAmount

@Olshansk Without making the proposed change the make develop_test works fine but this individual test fails, but after making changes this test passes but make develop_test fails. I am bit confused.

[Olshansk]

We identified the root cause of the issue in #472 so this lgtm!