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.

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

Fix flaky end-to-end transfer, protocol unit tests #1734

Merged
merged 22 commits into from
Nov 15, 2019
Merged

Conversation

asaj
Copy link
Contributor

@asaj asaj commented Nov 14, 2019

Description

This PR fixes three flaky tests:

  • The end-to-end transfer test, in which we do not always estimate gas correctly for setting the inflation params, causing our transaction to revert and the tests to fail.
  • The epoch rewards unit tests, in which we have tests that depend on block time but we are not traveling precisely to the correct block time
  • The stabletoken unit tests, which is checking the wrong value in one of the events

Tested

Tests pass

Other changes

  • Minor typo

@asaj asaj self-assigned this Nov 14, 2019
@asaj asaj changed the title [WIP] Debug transfer test flake Restart with clean nodes before every sync mode in demurrage transfer tests Nov 15, 2019
@asaj asaj assigned mcortesi and nategraf and unassigned asaj and mcortesi Nov 15, 2019
@asaj asaj changed the title Restart with clean nodes before every sync mode in demurrage transfer tests Fix flaky end-to-end transfer, protocol unit tests Nov 15, 2019
@asaj asaj requested a review from m-chrzan as a code owner November 15, 2019 03:42
@codecov
Copy link

codecov bot commented Nov 15, 2019

Codecov Report

Merging #1734 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1734   +/-   ##
=======================================
  Coverage   74.26%   74.26%           
=======================================
  Files         277      277           
  Lines        7617     7617           
  Branches      669      669           
=======================================
  Hits         5657     5657           
  Misses       1845     1845           
  Partials      115      115
Flag Coverage Δ
#mobile 74.26% <ø> (ø) ⬆️

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 0049e92...c60a447. Read the comment docs.

targetEpochReward = await epochRewards.getTargetEpochRewards()
targetEpochReward = targetEpochReward.plus(
await epochRewards.getTargetTotalEpochPaymentsInGold()
)
})

const timeTravelToDelta = async () => {
const currentTime = new BigNumber((await web3.eth.getBlock('latest')).timestamp)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is time that large that we require BigNumber?

@asaj asaj merged commit 1700119 into master Nov 15, 2019
aaronmgdr added a commit that referenced this pull request Nov 15, 2019
* master: (81 commits)
  Make styling more consistent in validator quick start and add password prompt to account:unlock (#1731)
  New Version for contractkit (#1727)
  Remove end-to-end attestations test from circle (#1739)
  Added helper function to get the list of current validators (#1713)
  Fix flaky end-to-end transfer, protocol unit tests (#1734)
  Add AccountClaim to Metadata (#1663)
  [Wallet] Set default gradle properties (#1629)
  Update genesis block after adding parent signatures to block header (#1732)
  Removed end-to-end-geth-integration-sync-test job in workflow (#1730)
  Change the event we emit when selecting issuers (#1706)
  [Wallet] Prompt user to restart app when turning off Forno a second time (#1708)
  Check in spanish verification translations (#1726)
  [Wallet] Style fixes on iOS for verification and backup flows (#1718)
  [Wallet] Reset isRedeemingInvite on rehydrate (#1716)
  Adjust e2e transfer and governance tests to match new fee distribution and eliminate ProposerFraction (#1585)
  [Wallet] Add more local currencies (#1698)
  Switch to correct cluster when fauceting (#1687)
  [Wallet] Use the country of the phone number for determining the default local currency (#1684)
  [Wallet] Limit QR code scanner to 1 code per second (#1676)
  Update Dark backgrounds text color (#1677)
  ...

# Conflicts:
#	packages/web/pages/_app.tsx
#	packages/web/src/dev/BuildPage.tsx
#	packages/web/src/dev/Cover.tsx
#	packages/web/src/dev/StackSection.tsx
#	yarn.lock
aaronmgdr added a commit that referenced this pull request Nov 26, 2019
* master: (70 commits)
  Make styling more consistent in validator quick start and add password prompt to account:unlock (#1731)
  New Version for contractkit (#1727)
  Remove end-to-end attestations test from circle (#1739)
  Added helper function to get the list of current validators (#1713)
  Fix flaky end-to-end transfer, protocol unit tests (#1734)
  Add AccountClaim to Metadata (#1663)
  [Wallet] Set default gradle properties (#1629)
  Update genesis block after adding parent signatures to block header (#1732)
  Removed end-to-end-geth-integration-sync-test job in workflow (#1730)
  Change the event we emit when selecting issuers (#1706)
  [Wallet] Prompt user to restart app when turning off Forno a second time (#1708)
  Check in spanish verification translations (#1726)
  [Wallet] Style fixes on iOS for verification and backup flows (#1718)
  [Wallet] Reset isRedeemingInvite on rehydrate (#1716)
  Adjust e2e transfer and governance tests to match new fee distribution and eliminate ProposerFraction (#1585)
  [Wallet] Add more local currencies (#1698)
  Switch to correct cluster when fauceting (#1687)
  [Wallet] Use the country of the phone number for determining the default local currency (#1684)
  [Wallet] Limit QR code scanner to 1 code per second (#1676)
  Update Dark backgrounds text color (#1677)
  ...
@mcortesi mcortesi deleted the asaj/e2e-transfer-flake branch December 4, 2019 16:48
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.

5 participants