Write test for AddEpochInfo #1895
Conversation
| // | ||
| // TODO: Make a new test for init genesis logic |
There was a problem hiding this comment.
InitGenesis basically just shells out to AddEpochInfo. Error on duplicate is tested in its TestEpochsInitGenesis already!
x/epochs/keeper/epoch_test.go
Outdated
| expErr bool | ||
| expEpochInfo types.EpochInfo | ||
| }{ | ||
| "simple_add": {addedEpochInfo: types.EpochInfo{ |
There was a problem hiding this comment.
I think formatting might be off here. Should this be on the next line?
There was a problem hiding this comment.
yeah whole tests fields' formatting would be better fixed before merging!
| @@ -6,6 +6,60 @@ import ( | |||
| "github.com/osmosis-labs/osmosis/v7/x/epochs/types" | |||
| ) | |||
|
|
|||
| func (suite *KeeperTestSuite) TestAddEpochInfo() { | |||
| defaultIdentifier := "default_add_epoch_info_id" | |||
There was a problem hiding this comment.
These might better be consts since these never change
There was a problem hiding this comment.
eh, I'm not sure I really buy into the notion that const vs not const matters for variables at the top of a test. If it was file-wide, I'd totally agree
| suite.Require().Equal(test.expEpochInfo, actualEpochInfo) | ||
| } else { | ||
| suite.Require().Error(err) | ||
| } |
There was a problem hiding this comment.
Are you planning to add more tests? I don't see one with an error expected
There was a problem hiding this comment.
imo, I think the only notable errror for AddEpochInfo is the case for returning an error when there's a duplicate epoch identifier, which is tested in the test below
x/epochs/keeper/epoch_test.go
Outdated
| expErr bool | ||
| expEpochInfo types.EpochInfo | ||
| }{ | ||
| "simple_add": {addedEpochInfo: types.EpochInfo{ |
There was a problem hiding this comment.
yeah whole tests fields' formatting would be better fixed before merging!
| suite.Require().Equal(test.expEpochInfo, actualEpochInfo) | ||
| } else { | ||
| suite.Require().Error(err) | ||
| } |
There was a problem hiding this comment.
imo, I think the only notable errror for AddEpochInfo is the case for returning an error when there's a duplicate epoch identifier, which is tested in the test below
Co-authored-by: Roman <[email protected]>
…osis into dev/add_epoch_info_test
|
Fixed formatting, added a zero duration test |
Closes: #1834
What is the purpose of the change
Adds a table driven test for AddEpochInfo functionality, and a unit test for its behavior on duplicates.
This is part of followup from #1893
Testing and Verifying
This change added tests
Documentation and Release Note
Unreleasedsection inCHANGELOG.md?no