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

Add tests for utils / setting up INI file #22

Merged
merged 9 commits into from
Apr 23, 2024
Merged

Add tests for utils / setting up INI file #22

merged 9 commits into from
Apr 23, 2024

Conversation

sacha-l
Copy link
Collaborator

@sacha-l sacha-l commented Apr 22, 2024

Unit tests that cover some items in #13 for the util functions for reading/writing to an INI file:

  • string_to_vec works
  • string_to_hashmap works
  • write_config works
  • read_ini_file works (with custom config and default fallback i.e. without ports or auth specified)

This PR also adds a few fixes to the library that were caught while writing tests, such as:

  1. generating a new keypair if none was read from the file
  2. fallback to default ports specified in prelude.rs if none were specified in the INI file
  3. rust fmt had the wrong extension 🙂‍↕️

Note, related to #7 and #15: we should let users know that BootstrapConfig will default to generate the keypairs in generate_keypair_from_protobuf even though the INI file doesn't have the required [auth] field. We may want to enforce that users have this compulsory field by panicking with a log message or we don't panic but log that the fallback was used. WDYT @thewoodfish ?

Please merge once reviewed =)

@sacha-l sacha-l added this to the milestone-1 milestone Apr 22, 2024
@sacha-l sacha-l requested a review from thewoodfish April 22, 2024 18:18
Copy link
Member

@thewoodfish thewoodfish left a comment

Choose a reason for hiding this comment

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

Great. I think we should let the [auth] section be optional and then log when it is absent, that we generated a default keypair and in turn a network identity.
The tests all work great.

@thewoodfish thewoodfish merged commit bbd3557 into main Apr 23, 2024
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.

2 participants