-
Notifications
You must be signed in to change notification settings - Fork 197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Internal block api unit testing #3695
Internal block api unit testing #3695
Conversation
api/groups/internalGroup_test.go
Outdated
expectedOutput := bytes.Repeat([]byte("1"), 10) | ||
|
||
facade := mock.FacadeStub{ | ||
GetInternalMetaBlockByHashCalled: func(_ common.OutportFormat, _ string) (interface{}, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please rename OutportFormat
to OutputFormat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
node/blockAPI/internalBlock_test.go
Outdated
storerMock := mock.NewStorerMock() | ||
uint64Converter := mock.NewNonceHashConverterMock() | ||
|
||
internalBlockProcessor := createMockInternalBlockProcessor( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might rename internalBlockProcessor
to ibp
as it has the same name as the struct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -154,7 +154,7 @@ func InitRatingsMetrics(statusHandlerUtils StatusHandlersUtils, ratingsConfig co | |||
appStatusHandler.SetStringValue(common.MetricRatingsMetaChainConsecutiveMissedBlocksPenalty, fmt.Sprintf("%f", ratingsConfig.MetaChain.ConsecutiveMissedBlocksPenalty)) | |||
|
|||
appStatusHandler.SetStringValue(common.MetricRatingsPeerHonestyDecayCoefficient, fmt.Sprintf("%f", ratingsConfig.PeerHonesty.DecayCoefficient)) | |||
appStatusHandler.SetUInt64Value(common.MetricRatingsPeerHonestyDecayCoefficient, uint64(ratingsConfig.PeerHonesty.DecayUpdateIntervalInSeconds)) | |||
appStatusHandler.SetUInt64Value(common.MetricRatingsPeerHonestyDecayUpdateIntervalInSeconds, uint64(ratingsConfig.PeerHonesty.DecayUpdateIntervalInSeconds)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
node/node.go
Outdated
@@ -1404,7 +1404,8 @@ func (n *Node) getKeyBytes(key string) ([]byte, error) { | |||
return hex.DecodeString(key) | |||
} | |||
|
|||
func (n *Node) createInternalBlockProcessor() error { | |||
// CreateInternalBlockProcessor creates the block processor for handling internal block header | |||
func (n *Node) CreateInternalBlockProcessor() error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you export it? if only for testing, then you'd better create an export_test.go
file where you export this function ONLY for testing purposes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, exported function in export_test.go
node/nodeInternalBlocks_test.go
Outdated
err := n.CreateInternalBlockProcessor() | ||
require.Nil(t, err) | ||
|
||
blk, err := n.GetInternalShardBlockByHash(common.Internal, "wronghashformat") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might rename Internal
from common
to ApiOutputFormatInternal
also rename Proto
from internal
to ApiOutputFormatProto
also rename OutportFormat
to ApiOutputFormat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done 👍
@@ -255,7 +255,7 @@ func (sm *statusMetrics) RatingsMetrics() map[string]interface{} { | |||
ratingsMetrics[common.MetricRatingsGeneralSignedBlocksThreshold] = sm.loadStringMetric(common.MetricRatingsGeneralSignedBlocksThreshold) | |||
|
|||
numSelectionChances := sm.loadUint64Metric(common.MetricRatingsGeneralSelectionChances + "_count") | |||
selectionChances := make([]interface{}, 0) | |||
selectionChances := make([]map[string]interface{}, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the items within the map seem only to be uint64
. therefore, we could change the map from map[string]interface{}
to map[string]uint64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right 👍
|
||
for i, selectionChance := range selectionChances { | ||
maxThresholdStr := fmt.Sprintf("%s%d%s", common.MetricRatingsGeneralSelectionChances, i, common.SelectionChancesMaxThresholdSuffix) | ||
sm.SetUInt64Value(maxThresholdStr, uint64(selectionChance["MaxThreshold"])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant type conversion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
maxThresholdStr := fmt.Sprintf("%s%d%s", common.MetricRatingsGeneralSelectionChances, i, common.SelectionChancesMaxThresholdSuffix) | ||
sm.SetUInt64Value(maxThresholdStr, uint64(selectionChance["MaxThreshold"])) | ||
chancePercentStr := fmt.Sprintf("%s%d%s", common.MetricRatingsGeneralSelectionChances, i, common.SelectionChancesChancePercentSuffix) | ||
sm.SetUInt64Value(chancePercentStr, uint64(selectionChance["ChancePercent"])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant type conversion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
- renaming - use valid hash in tests - rename OutportFormat type and vars - status handler - ratings metrics improvements
…twork/elrond-go into internal-block-api-unit-testing
3bd095f
to
3938e57
Compare
- handler errors properly - improve coverage
- remove duplicate error - handler error cases properly - reduce duplicate code
Unit testing on several parts regarding internal block headers exposure: