chore: improve feeburner test cov #ntrn-328 #130
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pr0n00gler
reviewed
Jan 20, 2023
pr0n00gler
reviewed
Jan 20, 2023
NeverHappened
changed the base branch from
main
to
neutron_audit_informal_17_01_2023
NeverHappened
force-pushed
the
chore/improve-feeburner-test-cov
branch
from
80405b4
to
5e3093e
Compare
NeverHappened
commented
Jan 20, 2023
| @@ -116,15 +116,15 @@ func (m *MockContractManagerKeeper) EXPECT() *MockContractManagerKeeperMockRecor | |||
| } | |||
|
|
|||
| // AddContractFailure mocks base method. | |||
| func (m *MockContractManagerKeeper) AddContractFailure(ctx types.Context, channelId, address string, ackID uint64, ackType string) { | |||
| func (m *MockContractManagerKeeper) AddContractFailure(ctx types.Context, channelID, address string, ackID uint64, ackType string) { | |||
There was a problem hiding this comment.
that's just gomock output
pr0n00gler
reviewed
Jan 25, 2023
x/feeburner/keeper/keeper.go
Outdated
| fundsForTreasury, | ||
| ) | ||
| if err != nil { | ||
| return fmt.Errorf("error sending funds to treasury for address=%s, tokens=%+v: %v", params.TreasuryAddress, fundsForTreasury, err) |
There was a problem hiding this comment.
What is the specific reason of returning an error here? We panic everywhere in this method except this particular line.
There was a problem hiding this comment.
That's a bit out of scope). But:
I thought the reason was that it's okay to skip sending to treasury in this block, because we'll do again in later blocks anyway.
But now I think your point is valid actually, and this is not an okay behaviour.
zavgorodnii
approved these changes
Feb 7, 2023
oldremez
approved these changes
Feb 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
improved coverage of feeburner.
to do that, needed to extract function from EndBlock
also changed to just panic if any error occurs during the process in EndBlock
https://github.com/neutron-org/neutron-tests/actions/runs/4084447916