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: add OverrideKmdConfig to libgoalFixture #6269

Merged

Conversation

algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Mar 7, 2025

Summary

Sometimes e2e tests fail with handle does not exist error. It ended up to be a wallet handle expiration issue: tests get a handle at the very beginning and keep using it till the end. Handle expiration is 60 seconds and a test can take longer. KMD starts a background thread that removes handles after expiration and there is no way to renew if it was removed.

To solve this I added a new KmdJSONOverride field to network templates (similar to ConfigJSONOverride) to set SessionLifetimeSecs to 5 minutes. At the moment modified only TwoNodes50Each.json and TwoNodes50EachFuture.json templates as the most common ones and ones with such failures.

Solved with adding OverrideKmdConfig to netdeploy and using it in libgoalFixture.
This solves e2e-go tests but not expect or sub-e2e tests.

Test Plan

Run some existing e2e tests locally.

Copy link

codecov bot commented Mar 7, 2025

Codecov Report

Attention: Patch coverage is 5.88235% with 32 lines in your changes missing coverage. Please review.

Project coverage is 51.76%. Comparing base (269945c) to head (e5f179a).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
netdeploy/networkTemplate.go 8.69% 21 Missing ⚠️
daemon/kmd/config/config.go 0.00% 8 Missing ⚠️
netdeploy/network.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6269      +/-   ##
==========================================
- Coverage   51.78%   51.76%   -0.02%     
==========================================
  Files         644      644              
  Lines       86697    86727      +30     
==========================================
- Hits        44894    44893       -1     
- Misses      38933    38966      +33     
+ Partials     2870     2868       -2     

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gmalouf
gmalouf previously approved these changes Mar 7, 2025
@jannotti
Copy link
Contributor

It seems strange that this becomes a property of the network, as opposed to a configuration when running KMD itself. They seem very different. Can our tests just start KMD is some special way? That way the .json files stay the same, and all the tests work.

@algorandskiy
Copy link
Contributor Author

Currently libgoalFixture uses netdeploy.CreateNetworkFromTemplate that takes a template. I guess it is possible to add another method and some non-exported properly on NetworkTemplate type to include store and apply kmd config values.

@algorandskiy
Copy link
Contributor Author

Okay, now I recall why I went this road. We cannot create KMD config from outside since it has kmd dir and it is not known unless template instantiation moment. So NetworkTemplate must have it is own type defining some subset of KMD config that needs to merged with the default one when KMD dir is known.

@algorandskiy
Copy link
Contributor Author

Implemented netdeploy's OverrideKmdConfig used in libgoalFixture.

@algorandskiy algorandskiy requested a review from gmalouf March 11, 2025 19:24
cce
cce previously approved these changes Mar 12, 2025
gmalouf
gmalouf previously approved these changes Mar 12, 2025
Copy link
Contributor

@gmalouf gmalouf left a comment

Choose a reason for hiding this comment

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

Looks like one comment wants to get in, otherwise looks fine.

jannotti
jannotti previously approved these changes Mar 12, 2025
@algorandskiy algorandskiy dismissed stale reviews from jannotti, gmalouf, and cce via e5f179a March 12, 2025 14:42
@algorandskiy algorandskiy changed the title netdeploy: add KmdJSONOverride test: add OverrideKmdConfig to libgoalFixture Mar 12, 2025
@algorandskiy algorandskiy merged commit 3a0cd6e into algorand:master Mar 12, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants