-
-
Notifications
You must be signed in to change notification settings - Fork 94
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] Improve tests #296
[FEAT] Improve tests #296
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #296 +/- ##
==========================================
- Coverage 57.04% 56.99% -0.05%
==========================================
Files 43 43
Lines 3997 3995 -2
==========================================
- Hits 2280 2277 -3
- Misses 1717 1718 +1
☔ View full report in Codecov by Sentry. |
settings.cashu_dir = "./test_data/" | ||
settings.mint_host = "localhost" | ||
settings.mint_port = SERVER_PORT | ||
settings.mint_host = "0.0.0.0" | ||
settings.mint_listen_port = SERVER_PORT | ||
settings.mint_url = SERVER_ENDPOINT | ||
settings.lightning = False | ||
settings.tor = False | ||
settings.mint_lightning_backend = "FakeWallet" | ||
settings.mint_database = "./test_data/test_mint" | ||
settings.mint_derivation_path = "0/0/0/0" | ||
settings.mint_private_key = "TEST_PRIVATE_KEY" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm, overwriting the settings like this strikes me as a bad idea -- to me intuitively settings should be immutable, avoids having value A at some time, then a bit of code runs, then it's now value B, things break or are confusing. Or potentially if you just import this file, it'll mangle the settings because the mutation doesn't happen in a function. Also as they're currently configured, they'll be type-checked on load (and will error if a setting is wrong), but they won't be when you re-assign them like this.
Instead it'd be 'better' to use a separate .env
for the tests (docs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its a hard override on settings for conftest. i think its fine, mixing up different .env
is error prone too. i could just prefix the poetry run command inside Makefile
but then running it with poetry run pytest
does not work.
everything works now for me locally.
for description look at my commit messages :)