Skip to content

Make block/epoch alignment in tests more predictable #555

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

Merged
merged 2 commits into from
Mar 30, 2022

Conversation

pcarranzav
Copy link
Member

(Temporarily targeting the pcv/fix-tests branch as this builds on #554)

Now that blocks are mined predictably in our unit tests (i.e. one block per transaction), any tests that rely on a specific number of blocks passing can break if the number of transactions changes. This was evident when I incorporated the changes from #554 into #552, see: https://github.com/graphprotocol/contracts/runs/5728132209?check_suite_focus=true - adding a transaction in the fixtures bringup caused the test to spill over into the next epoch, changing the results.

So this PR makes a few changes to make tests hopefully more predictable and robust:

  • Change the advanceToNextEpoch function to always advance to the first block in the next epoch, rather than always advancing a full epoch
  • Change several tests that advanced an epochLength to use advanceToNextEpoch instead
  • In takeRewards tests that expect an amount for indexer rewards, align to the first block in an epoch and recalculate the expected rewards using the number of blocks, that is now a bit easier to calculate by just counting the number of transactions in the test code.

@pcarranzav pcarranzav requested a review from abarmat March 29, 2022 19:42
Copy link
Contributor

@abarmat abarmat left a comment

Choose a reason for hiding this comment

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

Great improvements ⚡️

Base automatically changed from pcv/fix-tests to dev March 30, 2022 16:24
@pcarranzav pcarranzav closed this Mar 30, 2022
@pcarranzav pcarranzav reopened this Mar 30, 2022
@pcarranzav
Copy link
Member Author

(Closed and reopened to trigger CI)

@codecov
Copy link

codecov bot commented Mar 30, 2022

Codecov Report

Merging #555 (e38e530) into dev (3538e00) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev     #555   +/-   ##
=======================================
  Coverage   90.58%   90.58%           
=======================================
  Files          37       37           
  Lines        1795     1795           
  Branches      293      293           
=======================================
  Hits         1626     1626           
  Misses        169      169           
Flag Coverage Δ
unittests 90.58% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87a2468...e38e530. Read the comment docs.

@pcarranzav pcarranzav merged commit 2bd19f0 into dev Mar 30, 2022
@pcarranzav pcarranzav deleted the pcv/test-block-alignment branch March 30, 2022 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants