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.

Sign up for GitHub

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

Jump to bottom

refactor: adapt AddressBookTestingTool to support transaction Bytes #17197

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

refactor: adapt AddressBookTestingTool to support transaction Bytes #17197

wants to merge 9 commits into from
+404 −8

Conversation

IvanKavaldzhiev
Copy link
Collaborator

Description:
This PR enhances AddressBookTestingTool to utilize Bytes representation of system transactions.

Related issue(s):

Fixes #17081

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@IvanKavaldzhiev
refactor: adapt AddressBookTestingTool to support transaction Bytes
649f22c 
Signed-off-by: Ivan Kavaldzhiev <[email protected]>
@IvanKavaldzhiev
Merge remote-tracking branch 'origin/develop' into 17081-adapt-Addres…
2c6992b 
…sBookTestingTool-to-work-with-bytes

# Conflicts:
#	platform-sdk/platform-apps/tests/AddressBookTestingTool/src/main/java/com/swirlds/demo/addressbook/AddressBookTestingToolState.java

Signed-off-by: Ivan Kavaldzhiev <[email protected]>
@IvanKavaldzhiev
Merge remote-tracking branch 'origin/main' into 17081-adapt-AddressBo…
df28661 
…okTestingTool-to-work-with-bytes
@IvanKavaldzhiev
refactor: use Transaction interface instead of ConsensusTransaction
d0b9c92 
Signed-off-by: Ivan Kavaldzhiev <[email protected]>
@IvanKavaldzhiev
Merge remote-tracking branch 'origin/main' into 17081-adapt-AddressBo…
83e87c7 
…okTestingTool-to-work-with-bytes
@IvanKavaldzhiev IvanKavaldzhiev added the Platform Tickets pertaining to the platform label Jan 3, 2025
@IvanKavaldzhiev IvanKavaldzhiev added this to the v0.59 milestone Jan 3, 2025
@IvanKavaldzhiev IvanKavaldzhiev self-assigned this Jan 3, 2025
@IvanKavaldzhiev IvanKavaldzhiev requested review from a team as code owners January 3, 2025 10:21
@IvanKavaldzhiev
style: spotless apply
1ac1340 
Signed-off-by: Ivan Kavaldzhiev <[email protected]>
@codecov Codecov
Copy link

codecov bot commented Jan 3, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 64.43%. Comparing base (59be979) to head (f7e326e).

Files with missing lines Patch % Lines
...s/demo/addressbook/AddressBookTestingToolMain.java 0.00% 1 Missing ⚠️
...n/java/com/swirlds/platform/system/SwirldMain.java 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main   #17197      +/-   ##
============================================
+ Coverage     64.34%   64.43%   +0.09%     
- Complexity    20945    20966      +21     
============================================
  Files          2555     2555              
  Lines         96169    96181      +12     
  Branches      10054    10054              
============================================
+ Hits          61877    61972      +95     
+ Misses        30654    30557      -97     
- Partials       3638     3652      +14
Files with missing lines Coverage Δ
.../demo/addressbook/AddressBookTestingToolState.java 21.21% <100.00%> (+21.21%) ⬆️
...s/demo/addressbook/AddressBookTestingToolMain.java 22.72% <0.00%> (+22.72%) ⬆️
...n/java/com/swirlds/platform/system/SwirldMain.java 0.00% <0.00%> (ø)

... and 9 files with indirect coverage changes

Impacted file tree graph

@codacy-production Codacy Production
Copy link

codacy-production bot commented Jan 3, 2025 •
edited
Loading

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
✅ +0.11% (target: -1.00%) ✅ 87.50%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (59be979) 95932 65323 68.09%
Head commit (f7e326e) 95944 (+12) 65432 (+109) 68.20% (+0.11%)

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 (#17197) 16 14 87.50%

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

andrewb1269hg
andrewb1269hg approved these changes Jan 3, 2025
View reviewed changes
Copy link
Contributor

@andrewb1269hg andrewb1269hg left a comment

Choose a reason for hiding this comment

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

Review and approve file platform-sdk/platform-apps/tests/AddressBookTestingTool/build.gradle.kts

andrewb1269hg
andrewb1269hg previously approved these changes Jan 6, 2025
View reviewed changes
Copy link
Contributor

@andrewb1269hg andrewb1269hg left a comment

Choose a reason for hiding this comment

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

Review and approve file platform-sdk/platform-apps/tests/AddressBookTestingTool/build.gradle.kts

mustafauzunn
mustafauzunn reviewed Jan 7, 2025
View reviewed changes
...kTestingTool/src/test/java/com/swirlds/demo/addressbook/AddressBookTestingToolStateTest.java Outdated

package com.swirlds.demo.addressbook;

import static com.swirlds.platform.test.fixtures.state.FakeMerkleStateLifecycles.FAKE_MERKLE_STATE_LIFECYCLES;
Copy link
Collaborator

Choose a reason for hiding this comment

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

FakeMerkleStateLifecycles was renamed to FakeStateLifecycles
so we need to change the import to
import static com.swirlds.platform.test.fixtures.state.FakeStateLifecycles.FAKE_MERKLE_STATE_LIFECYCLES;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, I renamed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mustafauzunn mustafauzunn mustafauzunn left review comments

@andrewb1269hg andrewb1269hg andrewb1269hg left review comments

@lpetrovic05 lpetrovic05 Awaiting requested review from lpetrovic05

@poulok poulok Awaiting requested review from poulok

@rbarkerSL rbarkerSL Awaiting requested review from rbarkerSL rbarkerSL is a code owner automatically assigned from hashgraph/devops-ci

At least 1 approving review is required to merge this pull request.

Assignees

@IvanKavaldzhiev IvanKavaldzhiev

Labels
Platform Tickets pertaining to the platform
Projects
None yet
Milestone
v0.59
Development

Successfully merging this pull request may close these issues.

Adapt AddressBookTestingTool to work with the Bytes format for system transactions
3 participants
@IvanKavaldzhiev @mustafauzunn @andrewb1269hg