Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Unit tests for MixinCumulativeRewards #2316

Merged
merged 2 commits into from
Nov 6, 2019

Conversation

hysz
Copy link
Contributor

@hysz hysz commented Nov 5, 2019

Description

Unit tests for MixinCumulativeRewards in the Staking contracts package.

Testing instructions

Types of changes

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

@hysz hysz requested a review from moodlezoup November 5, 2019 17:15
@hysz hysz requested a review from abandeali1 as a code owner November 5, 2019 17:15
@hysz hysz force-pushed the tests/3.0/StakingMixinCumulativeRewardsUnitTEsts branch from e1f0708 to 4881ef9 Compare November 5, 2019 18:21
@coveralls
Copy link

coveralls commented Nov 5, 2019

Coverage Status

Coverage remained the same at 75.286% when pulling 2d0ad6f on tests/3.0/StakingMixinCumulativeRewardsUnitTEsts into a902235 on development.

Copy link
Contributor

@jalextowle jalextowle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good! Just one thought and then g2g

expect(reward).to.bignumber.equal(ZERO);
});

it('Should revert if start is greater than the end of the interval', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite the fact that this should never happen, I think it could make sense to test for the case when we don't have the right "endpoints." Basically, when we are missing the cumulative reward values that we need to calculate rewards.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is handled by Should successfully compute reward are no cumulative reward entries - in this case the default CR is used.

@hysz hysz force-pushed the tests/3.0/StakingMixinCumulativeRewardsUnitTEsts branch from 4881ef9 to 2d0ad6f Compare November 5, 2019 23:34
@hysz hysz merged commit 44793a9 into development Nov 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants