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: solder test input wire for TurleTests to StateSignatureCollector output wire #17864

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

Conversation

IvanKavaldzhiev
Copy link
Contributor

Description:
This PR defines a test component inside TurtleTests and wires it to collect ReservedSignedStates coming from StateSignatureCollector.

Related issue(s):

Fixes #17639

Notes for reviewer:

Checklist

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

…sEngine

Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
…mponent-to-collect-rounds-from-ConsensusEngine
…mponent-to-collect-rounds-from-ConsensusEngine

# Conflicts:
#	platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/turtle/runner/TurtleNode.java

Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
…mponent-to-collect-rounds-from-ConsensusEngine
Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
…collect-rounds-from-ConsensusEngine' into 17639-connect-test-component-to-StateSignatureCollector-output
…r output wire

Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
…mponent-to-collect-rounds-from-ConsensusEngine
…collect-rounds-from-ConsensusEngine' into 17639-connect-test-component-to-StateSignatureCollector-output
@IvanKavaldzhiev IvanKavaldzhiev added Test Development Test Development Platform Tickets pertaining to the platform labels Feb 12, 2025
@IvanKavaldzhiev IvanKavaldzhiev self-assigned this Feb 12, 2025
@IvanKavaldzhiev IvanKavaldzhiev requested a review from a team as a code owner February 12, 2025 14:03
Copy link

codecov bot commented Feb 12, 2025

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.

Project coverage is 69.00%. Comparing base (06c0155) to head (8fff5a5).

Files with missing lines Patch % Lines
...lds/platform/builder/PlatformComponentBuilder.java 0.00% 10 Missing ⚠️
...va/com/swirlds/platform/wiring/PlatformWiring.java 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                                            Coverage Diff                                            @@
##             17638-connect-test-component-to-collect-rounds-from-ConsensusEngine   #17864      +/-   ##
=========================================================================================================
- Coverage                                                                  69.00%   69.00%   -0.01%     
- Complexity                                                                 23035    23037       +2     
=========================================================================================================
  Files                                                                       2654     2654              
  Lines                                                                      99668    99675       +7     
  Branches                                                                   10287    10287              
=========================================================================================================
+ Hits                                                                       68775    68779       +4     
- Misses                                                                     26993    27002       +9     
+ Partials                                                                    3900     3894       -6     
Files with missing lines Coverage Δ
...va/com/swirlds/platform/wiring/PlatformWiring.java 80.50% <0.00%> (-0.18%) ⬇️
...lds/platform/builder/PlatformComponentBuilder.java 31.85% <0.00%> (-0.48%) ⬇️

... and 12 files with indirect coverage changes

Impacted file tree graph

Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.01% (target: -1.00%) 0.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (06c0155) 99451 72514 72.91%
Head commit (8fff5a5) 99458 (+7) 72512 (-2) 72.91% (-0.01%)

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 (#17864) 11 0 0.00%

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

…mponent-to-collect-rounds-from-ConsensusEngine
…atformBuilder

Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
…mponent-to-collect-rounds-from-ConsensusEngine
Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
…mponent-to-collect-rounds-from-ConsensusEngine
Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
…mponent-to-collect-rounds-from-ConsensusEngine
…mponent-to-collect-rounds-from-ConsensusEngine
…collect-rounds-from-ConsensusEngine' into 17639-connect-test-component-to-StateSignatureCollector-output

# Conflicts:
#	platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/builder/PlatformComponentBuilder.java
#	platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/turtle/runner/TurtleNode.java

Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
@IvanKavaldzhiev IvanKavaldzhiev requested a review from a team as a code owner February 17, 2025 10:04
@IvanKavaldzhiev IvanKavaldzhiev added this to the v0.60 milestone Feb 17, 2025
Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
Base automatically changed from 17638-connect-test-component-to-collect-rounds-from-ConsensusEngine to main February 20, 2025 08:35
…mponent-to-StateSignatureCollector-output

# Conflicts:
#	platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/wiring/PlatformWiring.java
#	platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/turtle/runner/TurtleNode.java
#	platform-sdk/swirlds-platform-core/src/testFixtures/java/com/swirlds/platform/test/fixtures/turtle/consensus/ConsensusRoundsHolder.java
#	platform-sdk/swirlds-platform-core/src/testFixtures/java/com/swirlds/platform/test/fixtures/turtle/consensus/ConsensusRoundsListContainer.java

Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
Signed-off-by: Ivan Kavaldzhiev <ivankavaldzhiev@gmail.com>
* @return the wiring for the state signature collector
*/
@NonNull
public OutputWire<List<ReservedSignedState>> getStateSignatureCollectorWiring() {
Copy link
Contributor

Choose a reason for hiding this comment

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

we cannot use the output of the collector directly, it needs to go through a reserver first

Comment on lines +11 to +18
final List<ReservedSignedState> collectedSignedStates = new ArrayList<>();

@Override
public void interceptSignedStates(@NonNull final List<ReservedSignedState> signedStates) {
if (!signedStates.isEmpty()) {
collectedSignedStates.addAll(signedStates);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

we should not keep these signed states for the whole run, this could lead to a OOM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, I think this issue is also relevant in this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform Tickets pertaining to the platform Test Development Test Development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create input wire that takes StateSignatureCollector output wire values, used for TurtleTests
2 participants