Adjust e2e transfer and governance tests to match new fee distribution and eliminate ProposerFraction #1585
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
|
Marking |
Codecov Report
@@ Coverage Diff @@
## master #1585 +/- ##
=======================================
Coverage 74.45% 74.45%
=======================================
Files 279 279
Lines 7582 7582
Branches 664 664
=======================================
Hits 5645 5645
Misses 1823 1823
Partials 114 114
Continue to review full report at Codecov.
|
asaj
approved these changes
Nov 6, 2019
.circleci/config.yml
Outdated
| @@ -522,7 +522,7 @@ jobs: | |||
| command: | | |||
| set -e | |||
| cd packages/celotool | |||
| ./ci_test_transfers.sh checkout master | |||
| ./ci_test_transfers.sh checkout victor/gpm | |||
There was a problem hiding this comment.
I'm still not convinced it's better to repoint both PRs instead of just repointing one.
There was a problem hiding this comment.
My intention is to merge celo-org/celo-blockchain#563, point this back to master, wait for CI to pass, then merge this one. I simply want to see that the e2e tests pass (or fail) on this PR while the other is still pending.
| @@ -442,8 +443,8 @@ describe('Transfer tests', function(this: any) { | |||
| before(`start geth on sync: ${syncMode}`, () => startSyncNode(syncMode)) | |||
|
|
|||
| describe('Transfer CeloGold >', () => { | |||
| const GOLD_TRANSACTION_GAS_COST = 29180 | |||
| describe('gasCurrency = CeloGold >', () => { | |||
| const GOLD_TRANSACTION_GAS_COST = 30005 | |||
There was a problem hiding this comment.
It was out of date and being ignored. I adjusted each of the gas costs based on master then added a step step to enforce that they are accurate.
| expectedGas: 189456, | ||
| // TODO(nategraf): Initializing a stable token account costs 5k additional gas. | ||
| // The first time this transfer is done, which is for the full node, this will be seen. | ||
| expectedGas: syncMode === 'full' ? 190303 : 175303, |
There was a problem hiding this comment.
Can we restart with clean nodes so we don't have this funky check?
There was a problem hiding this comment.
Restarting with clean nodes takes longer and reduces test coverage because it only considers transfers to fresh accounts. I'll go ahead and split this off into it's own test case though.
added 12 commits
November 6, 2019 09:22
added 6 commits
November 11, 2019 15:15
aaronmgdr
added a commit
that referenced
this pull request
Nov 14, 2019
* master: (56 commits) 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) Remove integration sync test Minor attestation service fixes (#1680) [wallet] Fixed Native phone picker Use native API instead (#1669) Fix token addresses for notification service (#1674) Add golang to setup docs [wallet] Hide invite education copy after invite was redeemed (#1670) [Wallet] Add spinner, timer, and tip text to Verification input screen (#1656) [Wallet] Fix app deprecation check mechanism (#1358) Point end-to-end governance test back to master (#1665) Add EpochRewards smart contract to calculate epoch rewards and payments (#1558) Optimized Attestation view calls and removal of the reveal TX (#1578) Support claim signatures and support Keybase claims (#1575) [Wallet] Add timestamp to top banner messages (#1657) Export geth metrics on VM testnet (#1351) ... # Conflicts: # yarn.lock
aaronmgdr
added a commit
that referenced
this pull request
Nov 14, 2019
* master: 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) Remove integration sync test Minor attestation service fixes (#1680) Add golang to setup docs
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) ...
This was referenced Mar 2, 2022
This was referenced Sep 22, 2022
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.
Description
Fee distribution updated in celo-org/celo-blockchain#563 requires updates to the e2e transfer test. This PR provides those changes.
Tested
Tested with e2e and existing unit tests.
Other changes
Related issues
Backwards compatibility
Introduces a breaking change to contract kit by removing proposer fraction. Test changes are compatible with blockchain clients which include celo-org/celo-blockchain#563
Depends on celo-org/celo-blockchain#563