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

feat: use address book to determine node id #10453

Merged
merged 11 commits into from Dec 18, 2023

Conversation

kimbor
Copy link
Member

@kimbor kimbor commented Dec 12, 2023

Description:
Change the way that node id is determined at startup.

Related issue(s):

Fixes #10450

Signed-off-by: Kim Rader <kim.rader@swirldslabs.com>
Signed-off-by: Kim Rader <kim.rader@swirldslabs.com>
Copy link

github-actions bot commented Dec 13, 2023

Node: HAPI Test (Time Consuming) Results

21 tests     9 ✔️  25m 22s ⏱️
  2 suites  12 💤
  2 files      0

Results for commit 70fc065.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 13, 2023

Node: HAPI Test (Token) Results

189 tests  ±0   189 ✔️ ±0   17m 0s ⏱️ -57s
  13 suites ±0       0 💤 ±0 
  13 files   ±0       0 ±0 

Results for commit 70fc065. ± Comparison against base commit 50a7d5c.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 13, 2023

Node: HAPI Test (Crypto) Results

211 tests  ±0   204 ✔️ ±0   17m 7s ⏱️ -2s
  22 suites ±0       7 💤 ±0 
  22 files   ±0       0 ±0 

Results for commit 70fc065. ± Comparison against base commit 50a7d5c.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 13, 2023

Node: HAPI Test (Smart Contract) Results

398 tests  ±0   366 ✔️ +1   45m 33s ⏱️ +52s
  55 suites ±0     32 💤 ±0 
  55 files   ±0       0  - 1 

Results for commit 70fc065. ± Comparison against base commit 50a7d5c.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 13, 2023

Node: HAPI Test (Misc) Results

420 tests  ±0   329 ✔️ ±0   29m 25s ⏱️ +31s
  74 suites ±0     91 💤 ±0 
  74 files   ±0       0 ±0 

Results for commit 70fc065. ± Comparison against base commit 50a7d5c.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 13, 2023

Node: E2E Test Results

    1 files      1 suites   24m 7s ⏱️
311 tests 311 ✔️ 0 💤 0
333 runs  333 ✔️ 0 💤 0

Results for commit 70fc065.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 13, 2023

Node: Unit Test Results

    2 293 files  ±0      2 293 suites  ±0   51m 12s ⏱️ - 3m 57s
118 448 tests +3  118 414 ✔️ +3  34 💤 ±0  0 ±0 
126 865 runs  +3  126 831 ✔️ +3  34 💤 ±0  0 ±0 

Results for commit 70fc065. ± Comparison against base commit 50a7d5c.

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Dec 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (develop@50a7d5c). Click here to learn what that means.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop   #10453   +/-   ##
==========================================
  Coverage           ?   63.05%           
  Complexity         ?    30839           
==========================================
  Files              ?     3363           
  Lines              ?   135445           
  Branches           ?    14093           
==========================================
  Hits               ?    85409           
  Misses             ?    46676           
  Partials           ?     3360           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Dec 13, 2023

Node: Integration Test Results

278 tests  ±0   278 ✔️ ±0   26m 48s ⏱️ +3s
    5 suites ±0       0 💤 ±0 
    5 files   ±0       0 ±0 

Results for commit 70fc065. ± Comparison against base commit 50a7d5c.

♻️ This comment has been updated with latest results.

Signed-off-by: Nathan Klick <nathan@swirldslabs.com>
Signed-off-by: Nathan Klick <nathan@swirldslabs.com>
@kimbor kimbor marked this pull request as ready for review December 14, 2023 04:06
@kimbor kimbor requested review from a team as code owners December 14, 2023 04:06
jeromy-cannon
jeromy-cannon previously approved these changes Dec 14, 2023
nathanklick
nathanklick previously approved these changes Dec 14, 2023
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.

This scans to me; but imo is too critical a change to commit without unit test coverage.

@Neeharika-Sompalli
Copy link
Member

This scans to me; but imo is too critical a change to commit without unit test coverage.

➕ Agreed.

@nathanklick
Copy link
Member

This scans to me; but imo is too critical a change to commit without unit test coverage.

➕ Agreed.

There's a limit to how much we can unit test the main() method, but we can cover the supporting methods. However, each and every HapiTest execution does exert testing of these methods (for at least the happy path).

netopyr
netopyr previously approved these changes Dec 14, 2023
Copy link
Contributor

@netopyr netopyr left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Kim Rader <kim.rader@swirldslabs.com>
Signed-off-by: Kim Rader <kim.rader@swirldslabs.com>
…ine-startup-node-id' into 10450-use-address-book-to-determine-startup-node-id
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 @kimbor !

@netopyr netopyr merged commit 628226f into develop Dec 18, 2023
37 checks passed
@netopyr netopyr deleted the 10450-use-address-book-to-determine-startup-node-id branch December 18, 2023 16:45
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.

use address book to determine node id at startup
6 participants