Add Dockerfile with tests (async) #128
Conversation
| from typing import List | ||
| import unittest | ||
|
|
||
| from test.testing_common import ( |
There was a problem hiding this comment.
What's your motivation for changing the order there? I usually order such that the modules that are closest to the current code are coming last.
There was a problem hiding this comment.
Yes, so, both lndmanage.settings and test.testing_common have side-effects on import where they attempt to read/create the config file, and can error if this fails.
Before these changes, running the tests in a clean environment can fail due to this.
The idea here is that test.testing_common ensure that this does not happen by ensuring consistency and a default for LNDMANAGE_HOME. So testing_common needs to be imported before lndmanage.settings.
I think there was some discussions elsewhere on how it would be desirable to not have side-effects-by-import, which can be good to address down the line, but at least these changes (d788ceb in isolation) will ensure that the test suite doesn't interact with any existing config on the system.
| import shutil | ||
| from unittest import TestCase | ||
|
|
||
| # testing base folder |
There was a problem hiding this comment.
Also here, could you give me a hint why this is a better order for you? :)
There was a problem hiding this comment.
See above - os.environ.setdefault('LNDMANAGE_HOME', lndmanage_home) and os.makedirs before importing settings.
|
Pretty cool work, a big step towards CI! 🤖 |
2 participants
#125 based on the newer async version
Depends on lndmanage:local from bitromortac/lnregtest#7