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

test(db): add test for db #5463

Merged

Conversation

forfreeday
Copy link
Contributor

@forfreeday forfreeday commented Aug 31, 2023

What does this PR do?
add several test cases to cover more code verification scenarios

  1. add AccountAssetStoreTest
  2. add AssetIssueStoreTest
  3. add AssetIssueV2StoreTest
  4. add ContractStoreTest
  5. add DelegatedResourceStoreTest
  6. add DelegationStoreTest
  7. add WitnessScheduleStoreTest

Why are these changes required?
to improve test case coverage

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

private static final byte[] ASSET_KEY = "20000".getBytes();
private static AccountCapsule ownerCapsule;

private static String OWNER_ADDRESS = Wallet.getAddressPreFixString()
Copy link
Contributor

Choose a reason for hiding this comment

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

Use static final?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not necessary, and not using final doesn't hurt.



private long createAsset(String tokenName) {
long id = chainBaseManager.getDynamicPropertiesStore().getTokenIdNum() + 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this be conflict in any two test cases?

AssetIssueContractOuterClass.AssetIssueContract.newBuilder()
.setName(ByteString.copyFrom(firstTokenId.getBytes()))
.build());
assetIssueCapsule.setId(String.valueOf(2L));
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this not be the same as the token "efg"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

efg is the test data in testPut, and as this is a deleted test case, it makes sense not to use the same data


private SmartContractOuterClass.SmartContract.Builder createContract(
String contractAddress, String contractName) {
OWNER_ADDRESS =
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not return the contract?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no need to return contract, it is a static variable.

@halibobo1205 halibobo1205 mentioned this pull request Aug 29, 2023
5 tasks
@halibobo1205 halibobo1205 changed the title test(case): add test cases test(db): add test for db Sep 6, 2023
@halibobo1205 halibobo1205 added the area:test change in test label Sep 14, 2023
@forfreeday forfreeday changed the base branch from develop to release_v4.7.3 September 18, 2023 02:56
@forfreeday forfreeday merged commit 5e420e4 into tronprotocol:release_v4.7.3 Sep 18, 2023
2 checks passed
@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.7.3 milestone Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:test change in test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants