finish unit testing of consu addition legacy prop getter

cosmos · Nov 14, 2023 · a240ab5 · a240ab5
1 parent b26b453
commit a240ab5
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 51 deletions.
diff --git a/x/ccv/provider/keeper/hooks.go b/x/ccv/provider/keeper/hooks.go
@@ -224,6 +224,8 @@ func (h Hooks) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr s
 func (h Hooks) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) {
 }
 
+// GetConsumerAdditionLegacyPropFromProp extracts a consumer addition legacy proposal from
+// the proposal with given ID
 func (h Hooks) GetConsumerAdditionLegacyPropFromProp(
 	ctx sdk.Context,
 	proposalID uint64,
@@ -234,7 +236,7 @@ func (h Hooks) GetConsumerAdditionLegacyPropFromProp(
 	}
 
 	// Iterate over the messages in the proposal
-	// Note that only ONE message can contain a consumer addition proposal
+	// Note that it's assumed that at most ONE message can contain a consumer addition proposal
 	for _, msg := range p.GetMessages() {
 		sdkMsg, isLegacyProposal := msg.GetCachedValue().(*v1.MsgExecLegacyContent)
 		if !isLegacyProposal {

diff --git a/x/ccv/provider/keeper/hooks_test.go b/x/ccv/provider/keeper/hooks_test.go
@@ -91,7 +91,7 @@ func TestValidatorConsensusKeyInUse(t *testing.T) {
 	}
 }
 
-func TestAfterSubmissionAndAfterProposalVotingPeriodEnded(t *testing.T) {
+func TestAfterPropSubmissionAndVotingPeriodEnded(t *testing.T) {
 	acct := cryptotestutil.NewCryptoIdentityFromIntSeed(0)
 
 	propMsg, err := v1.NewLegacyContent(
@@ -142,48 +142,40 @@ func TestGetConsumerAdditionLegacyPropFromProp(t *testing.T) {
 		Amount:      sdk.NewCoins(sdk.NewCoin("stake", math.OneInt())),
 	}
 
+	textProp, err := v1.NewLegacyContent(
+		v1beta1.NewTextProposal("a title", "a legacy text prop"),
+		authtypes.NewModuleAddress("gov").String(),
+	)
+	require.NoError(t, err)
+
+	consuProp, err := v1.NewLegacyContent(
+		testkeeper.GetTestConsumerAdditionProp(),
+		authtypes.NewModuleAddress("gov").String(),
+	)
+	require.NoError(t, err)
+
 	testCases := map[string]struct {
-		propMsg func() sdk.Msg
+		propMsg sdk.Msg
 		// setup 			func(sdk.Context, k providerkeeper, proposalID uint64)
 		expPanic        bool
 		expConsuAddProp bool
 	}{
-
 		"prop not found": {
-			propMsg: func() sdk.Msg {
-				return dummyMsg
-			},
+			propMsg:  nil,
 			expPanic: true,
 		},
 		"msgs in prop contain no legacy props": {
-			propMsg: func() sdk.Msg {
-				return dummyMsg
-			},
-			expConsuAddProp: false,
+			propMsg: dummyMsg,
 		},
 		"msgs contain a legacy prop but not of ConsumerAdditionProposal type": {
-			propMsg: func() sdk.Msg {
-				textProp, err := v1.NewLegacyContent(
-					v1beta1.NewTextProposal("a title", "a legacy text prop"),
-					authtypes.NewModuleAddress("gov").String(),
-				)
-				require.NoError(t, err)
-
-				return textProp
-			},
-			expConsuAddProp: false,
+			propMsg: textProp,
+		},
+		"msgs contain an invalid legacy prop": {
+			propMsg:  &v1.MsgExecLegacyContent{},
+			expPanic: true,
 		},
 		"msg contains a prop of ConsumerAdditionProposal type - hook should create a new proposed chain": {
-			propMsg: func() sdk.Msg {
-				// create a dummy consumer addition prop
-				consuProp, err := v1.NewLegacyContent(
-					testkeeper.GetTestConsumerAdditionProp(),
-					authtypes.NewModuleAddress("gov").String(),
-				)
-				require.NoError(t, err)
-
-				return consuProp
-			},
+			propMsg:         consuProp,
 			expConsuAddProp: true,
 		},
 	}
@@ -193,30 +185,33 @@ func TestGetConsumerAdditionLegacyPropFromProp(t *testing.T) {
 			k, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
 			defer ctrl.Finish()
 
-			// create a dummy prop for each test case
-			prop, err := v1.NewProposal(
-				[]sdk.Msg{tc.propMsg()},
-				0,
-				time.Now(),
-				time.Now(),
-				"",
-				"",
-				"",
-				sdk.AccAddress(acct.SDKValOpAddress()),
+			var (
+				prop      v1.Proposal
+				propFound bool
 			)
-			require.NoError(t, err)
 
-			if !tc.expPanic {
-				// pass the prop to the mocked gov keeper,
-				// which is called by the AfterProposalSubmission gov hook
-				gomock.InOrder(
-					mocks.MockGovKeeper.EXPECT().GetProposal(ctx, prop.Id).Return(prop, true),
-				)
-			} else {
-				gomock.InOrder(
-					mocks.MockGovKeeper.EXPECT().GetProposal(ctx, prop.Id).Return(v1.Proposal{}, false),
+			if tc.propMsg != nil {
+				propFound = true
+				prop, err = v1.NewProposal(
+					[]sdk.Msg{tc.propMsg},
+					0,
+					time.Now(),
+					time.Now(),
+					"",
+					"",
+					"",
+					sdk.AccAddress(acct.SDKValOpAddress()),
 				)
+				require.NoError(t, err)
+			}
+
+			gomock.InOrder(
+				mocks.MockGovKeeper.EXPECT().GetProposal(ctx, prop.Id).Return(prop, propFound),
+			)
+
+			if tc.expPanic {
 				defer func() {
+					// fail test if not panic was recovered
 					if r := recover(); r == nil {
 						require.Fail(t, r.(string))
 					}