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

Add Dynamic Address Book unit tests #2459

Merged
merged 19 commits into from
Aug 20, 2024
Merged

Add Dynamic Address Book unit tests #2459

merged 19 commits into from
Aug 20, 2024

Conversation

ivaylonikolov7
Copy link
Contributor

@ivaylonikolov7 ivaylonikolov7 commented Aug 16, 2024 •
edited
Loading

Description:

  • Adds missing unit tests for Dynamic Address Book
  • Getter naming convention should be consistent with other classes
  • Setters should update only when frozen
  • NodeId was missing when we make tx body for NodeUpdateTransaction

Related issue(s):
#2406

Fixes #
#2406

Notes for reviewer:

Checklist

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

@ivaylonikolov7
chore: getter naming convention
d32de07 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
fix: setters should work only when frozen
de4244c 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
fix: NodeUpdateTransaction was missing nodeid in tx body
4ec0d76 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test(wip): DAB unit tests
9c8468f 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: change byte test names
541be33 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: use ED25519 instead of depricated methods
0f7af75 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: dont use async functions and await when not necessary
6d328ec 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: rename typo test name
0ec09b8 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: use naming convention camelCase
0ea547c 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: add comment for variable clarity
cb51d65 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: no need for TEST prefix
d3f145e 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
update describe block names
1540922 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7 ivaylonikolov7 marked this pull request as ready for review August 18, 2024 20:55
@ivaylonikolov7 ivaylonikolov7 requested review from a team as code owners August 18, 2024 20:55
@ivaylonikolov7 ivaylonikolov7 self-assigned this Aug 19, 2024
@ivaylonikolov7 ivaylonikolov7 added this to the v2.50.0 milestone Aug 19, 2024
gsstoykov
gsstoykov reviewed Aug 19, 2024
View reviewed changes
test/unit/NodeCreateTransaction.js Show resolved Hide resolved
test/unit/NodeCreateTransaction.js Show resolved Hide resolved
test/unit/NodeCreateTransaction.js Outdated Show resolved Hide resolved
test/unit/NodeUpdateTransaction.js Show resolved Hide resolved
test/unit/NodeUpdateTransaction.js Show resolved Hide resolved
test/unit/NodeCreateTransaction.js Outdated Show resolved Hide resolved
test/unit/NodeCreateTransaction.js Outdated Show resolved Hide resolved
@ivaylonikolov7
test: use const as error message
38f2ebe 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: remove wrong error message
87ff8c5 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: add gossip and service endpoints tests for from-to bytes test
04abebc 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: dont use join for endpoints
c030412 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: use underscore because we dont use element
027b9c8 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: use error message for frozen tests as constant
443cccb 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@ivaylonikolov7
test: try catch consistency
02836ea 
Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.7% Duplication on New Code

See analysis details on SonarCloud

0xivanov
0xivanov reviewed Aug 20, 2024
View reviewed changes
Copy link
Contributor

@0xivanov 0xivanov left a comment

Choose a reason for hiding this comment

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

You can try adding tests for the protobuf related functions from/to protobuf.
See TokenRejectTransaction unit tests.

gsstoykov
gsstoykov reviewed Aug 20, 2024
View reviewed changes
Copy link
Contributor

@gsstoykov gsstoykov left a comment

Choose a reason for hiding this comment

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

I agree with Vanko. Maybe we should merge after adding those tests.

@ivaylonikolov7
Copy link
Contributor Author

We have tests that check the fromBytes/toBytes that basically check if it correctly decoded the protobufs.

@ivaylonikolov7 ivaylonikolov7 merged commit 05d3c9c into main Aug 20, 2024
7 checks passed
@ivaylonikolov7 ivaylonikolov7 deleted the test/add-missing-dab-unit-tests branch August 20, 2024 11:42
ivaylogarnev-limechain pushed a commit that referenced this pull request Aug 22, 2024
@ivaylonikolov7 @ivaylogarnev-limechain
Add Dynamic Address Book unit tests (#2459)
72c9bac 
* chore: getter naming convention

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* fix: setters should work only when frozen

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* fix: NodeUpdateTransaction was missing nodeid in tx body

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test(wip): DAB unit tests

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: change byte test names

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: use ED25519 instead of depricated methods

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: dont use async functions and await when not necessary

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: rename typo test name

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: use naming convention camelCase

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: add comment for variable clarity

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: no need for TEST prefix

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* update describe block names

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: use const as error message

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: remove wrong error message

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: add gossip and service endpoints tests for from-to bytes test

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: dont use join for endpoints

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: use underscore because we dont use element

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: use error message for frozen tests as constant

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

* test: try catch consistency

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>

---------

Signed-off-by: Ivaylo Nikolov <ivaylo.nikolov@limechain.tech>
This was referenced Aug 22, 2024
Merged
Closed
@ivaylonikolov7 ivaylonikolov7 linked an issue Aug 23, 2024 that may be closed by this pull request
Add unit tests for HIP-869 #2406
Closed
@si618 si618 mentioned this pull request Oct 4, 2024
Open
@hedera-eng-infrastructure hedera-eng-infrastructure mentioned this pull request Oct 4, 2024
Closed
@swirlds-automation swirlds-automation mentioned this pull request Oct 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsstoykov gsstoykov gsstoykov approved these changes

@0xivanov 0xivanov 0xivanov approved these changes

@rwalworth rwalworth Awaiting requested review from rwalworth rwalworth was automatically assigned from hashgraph/hedera-sdk

Assignees

@ivaylonikolov7 ivaylonikolov7

Labels
None yet
Projects
None yet
Milestone
v2.50.0
Development

Successfully merging this pull request may close these issues.

Add unit tests for HIP-869
3 participants
@ivaylonikolov7 @0xivanov @gsstoykov