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: New HAPI test for TokenAirdrop transaction #15348

Merged

Conversation

Evdokia-Georgieva
Copy link
Contributor

@Evdokia-Georgieva Evdokia-Georgieva commented Sep 5, 2024

Description:
This PR adds one test case to TokenAirdropTest.java

The test case extends AIRDROP_27 with max transfers, max tokens, max receivers, max fee collectors in order to ensure that the transaction limit is not reached.

This PR also modifies TokenAirdropBase.java -> createAccountsAndTokensWithAllCustomFees() is added

Test Plan:
https://www.notion.so/TokenAirdropTransaction-Test-Plan-919ed1800ee845378c5c625648d05ebc?pvs=4

Resolves

  1. Extend test AIRDROP - 27 with max transfers, max tokens, max receivers, max fee collectors so that we ensure we won't hit the transaction limit #15301

Copy link

codacy-production bot commented Sep 5, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.00% (target: -1.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (c7f12c7) 108843 67060 61.61%
Head commit (3ff26d0) 108843 (+0) 67060 (+0) 61.61% (+0.00%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#15348) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Copy link

codecov bot commented Sep 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.06%. Comparing base (c7f12c7) to head (3ff26d0).
Report is 33 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             develop   #15348   +/-   ##
==========================================
  Coverage      58.06%   58.06%           
  Complexity     21526    21526           
==========================================
  Files           2775     2775           
  Lines         109026   109026           
  Branches       11171    11171           
==========================================
  Hits           63307    63307           
  Misses         41856    41856           
  Partials        3863     3863           

Impacted file tree graph

Copy link
Collaborator

@tinker-michaelj tinker-michaelj left a comment

Choose a reason for hiding this comment

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

Very systematic treatment @Evdokia-Georgieva , nice work!

Our style guide for @HapiTest's does recommend splitting large tests into multiple smaller tests that focus on very specific scenarios. 🙏

@Evdokia-Georgieva
Copy link
Contributor Author

Very systematic treatment @Evdokia-Georgieva , nice work!

Our style guide for @HapiTest's does recommend splitting large tests into multiple smaller tests that focus on very specific scenarios. 🙏

@tinker-michaelj in my latest commit I did my best to change the test according to the style guide, unfortunately I could not strictly fulfil the rule for maximum of 7 operations per test. I had to include a little bit more checks as in the test case I need to perform airdrop transaction with the maximum allowed 10 transfers and each transfer needs to have different fee collector account.
In order to shorten the test, I removed the checks for the receiver balances as they only confirm that the receiver still has none of the airdropped tokens. I assume that as we confirm that the pending airdrops are created then the receiver balance will be correct, but I think that it is important to check the fee collectors account balances with the transferred fees and this adds few more operations.
I hope that for this case we can make an exception and allow it to have these few more operations. Please let me know if you agree or I should find another solution.
Thank you in advance for your help!

Copy link
Member

@kimbor kimbor left a comment

Choose a reason for hiding this comment

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

lgtm; I don't see a way to split this large test up into smaller tests, as the whole point of this test is to test a multiple transfer scenario

Copy link
Collaborator

@tinker-michaelj tinker-michaelj left a comment

Choose a reason for hiding this comment

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

LGTM, tyvm @Evdokia-Georgieva !

@JivkoKelchev
Copy link
Contributor

LGTM, thank you @Evdokia-Georgieva

Copy link
Member

@Neeharika-Sompalli Neeharika-Sompalli left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @Evdokia-Georgieva

@Evdokia-Georgieva Evdokia-Georgieva merged commit 729ae99 into develop Sep 19, 2024
53 checks passed
@Evdokia-Georgieva Evdokia-Georgieva deleted the 14366-add-new-TokenAirdropTest-tests-Airdrop_27 branch September 19, 2024 06:59
netopyr added a commit that referenced this pull request Sep 19, 2024
* develop: (22 commits)
  test: New HAPI test for TokenAirdrop transaction (#15348)
  test: unit test verifySyncInvalidEd25519() is not stable (#15534)
  chore: add `TracerBinding` interface for `TransactionExecutors`. (#15480)
  chore: Add missing javadocs in Consensus Service (#15299)
  fix: Validate `CustomFees` input arrays in `UpdateTokenCustomFeesDecoder` (#15520)
  feat: migrate event serialization to protobuf (#15417)
  fix: 15494: Improve VirtualLeafRecord serialization to bytes during flushes (#15512)
  docs: tss block signing proposal (#15160)
  fix: 15438: Eliminate busy loop in HalfDiskHashMap.endWriting() (#15439)
  build: cleanup settings.gradle.kts / remove build.gradle.kts (#15470)
  test: fix CryptographyTests (#15529)
  fix: recreate block hash from state (#15444)
  docs: Proposal Process Update - Specify post-acceptance non-material update procedure (#15447)
  chore: testnet event hashing (#15432)
  fix: 15167: Remove timeout from reconnect/rehash Iterators (#15468)
  chore: remove snapshot ops (#15462)
  feat: Add TokenUpdateNFTs as a smart contract operation v2 (#15445)
  feat: introduce PbjRecordHasher and RosterUtils.hash(Roster) (#15457)
  fix: Precision loss for gas calculation of HTS system contracts v2 (#15446)
  chore: correct the variable name in roster.proto (#15465)
  ...

# Conflicts:
#	hedera-node/hedera-smart-contract-service-impl/src/main/java/com/hedera/node/app/service/contract/impl/exec/scope/HandleHederaOperations.java
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.

6 participants