This repository was archived by the owner on Jul 23, 2024. It is now read-only.
Updated test cases which should works when Klaytn uses dynamic gas price policy #329
Conversation
This file contains hidden or 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
When fill gasPrice, the price should be enabled to change based on Klaytn network. Klaytn will apply dynamic gasPrice policy, so need to preapre for it.
If we don't remove get keyword, that method is called during serializing/de-serealizing process. So we shouldn't use get keyword in front of the method name
This class is used in codegen package, so it should not be deprecated currently
It should be attached
We should confirm that legacy test works fine when Klaytn activate dynamic gas price policy. For this, gas price should be fetched by using gasProvider.getGasPrice().
We should confirm that legacy test works fine when Klaytn activate dynamic gas price policy. For this, gas price should be fetched by using gasProvider.getGasPrice().
Both should work when Klaytn uses Dynamic fee policy.
For this, I changed unit price of Klaytn which is not 25 ston.
1. Updated legacy test cases
We should confirm that legacy test works fine when Klaytn activate dynamic gas price policy.
For this, gas price should be fetched by using gasProvider.getGasPrice().
And also increase gas limit because some test cases on recent Klaytn node requires more gas.
2. Updated recent test cases
We need to use caver.transaction.${txType}.create to create transaction instance because now we need to fetch dynamic gas price and set with that value.
… updates-legacy-testcases-0 # Conflicts: # core/src/test/java/com/klaytn/caver/base/Accounts.java # core/src/test/java/com/klaytn/caver/legacy/feature/ManagedTransactionTest.java # core/src/test/java/com/klaytn/caver/legacy/scenario/AccountKeyIT.java # core/src/test/java/com/klaytn/caver/legacy/scenario/Scenario.java
jimni1222
reviewed
Jul 5, 2022
core/src/test/java/com/klaytn/caver/legacy/feature/CustomGasTest.java
Outdated
Show resolved
Hide resolved
jimni1222
reviewed
Jul 5, 2022
core/src/test/java/com/klaytn/caver/legacy/feature/CustomGasTest.java
Outdated
Show resolved
Hide resolved
If we call gasProvider.getGasPrice every time, it can be changed because gasPrice depends on network status. During test, many txs can be processed and gasPrice can be dynamic.
jimni1222
reviewed
Jul 5, 2022
core/src/test/java/com/klaytn/caver/legacy/feature/CustomGasTest.java
Outdated
Show resolved
Hide resolved
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Proposed changes
Test cases should work after Klaytn applied Dynamic fee policy.
For this, I changed unit price of Klaytn which is not 25 ston.
1. Updated legacy test cases
We should confirm that legacy test works fine when Klaytn activate dynamic gas price policy.
For this, gas price should be fetched by using gasProvider.getGasPrice().
And also increase gas limit because some test cases on recent Klaytn node requires more gas.
2. Updated recent test cases
We need to use caver.transaction.${txType}.create to create transaction instance because now we need to fetch dynamic gas price and set with that value.
Types of changes
Please put an x in the boxes related to your change.
Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
Related issues
related to #321
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...