Skip to content
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.

Sign up for GitHub

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

Jump to bottom

test: Add specs for the EarningsInterface #223

Merged
merged 5 commits into from
Feb 23, 2022
Merged

test: Add specs for the EarningsInterface #223

merged 5 commits into from
Feb 23, 2022

Conversation

karelianpie
Copy link
Contributor

@karelianpie karelianpie commented Feb 14, 2022 •
edited
Loading

Partly addresses #211

Add the following tests for the EarningsInterface

  • protocolEarnings
  • assetEarnings
  • assetsHistoricEarnings

The remaining tests will be done in a separate PR for quicker review time following a discussion on Telegram with @xgambitox

  • accountAssetsData
  • assetHistoricEarnings
  • accountHistoricEarnings

@karelianpie karelianpie changed the title test: Initial test setup for EarningsInterface test: Add specs for the EarningsInterface Feb 15, 2022
@karelianpie karelianpie marked this pull request as ready for review February 16, 2022 04:32
xgambitox
xgambitox previously approved these changes Feb 16, 2022
View reviewed changes
@gotzenx gotzenx requested a review from jstashh February 21, 2022 18:08
@karelianpie karelianpie requested a review from FiboApe February 22, 2022 00:12
jstashh
jstashh approved these changes Feb 22, 2022
View reviewed changes
Copy link
Contributor

@jstashh jstashh left a comment

Choose a reason for hiding this comment

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

Great work, good to add these tests and thanks for the improvements to the actual code as well.

I think we're OK to merge this, see the comment about the dependency that autogenerates mocks, I think that'd be good to add in a future PR

src/services/subgraph/index.ts Show resolved Hide resolved
src/interfaces/earnings.ts Show resolved Hide resolved
src/interfaces/earnings.spec.ts Show resolved Hide resolved
@jstashh jstashh merged commit c5ac82e into yearn:master Feb 23, 2022
@karelianpie karelianpie deleted the issue-211 branch February 23, 2022 22:02
@karelianpie karelianpie self-assigned this Mar 22, 2022
jstashh pushed a commit that referenced this pull request May 31, 2022
@karelianpie @jstashh
test: Add specs for the EarningsInterface (#223)
8fcc523 
* test: Initial test setup for `EarningsInterface`

* test: Add spec for `protocolEarnings`

* test: Add spec for `assetEarnings`

* test: Add spec for `assetsHistoricEarnings`

* refactor: Extract asset address
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jstashh jstashh jstashh approved these changes

@gotzenx gotzenx Awaiting requested review from gotzenx

@xgambitox xgambitox Awaiting requested review from xgambitox

@FiboApe FiboApe Awaiting requested review from FiboApe

Assignees

@karelianpie karelianpie

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@karelianpie @xgambitox @jstashh @gotzenx @FiboApe