-
Notifications
You must be signed in to change notification settings - Fork 912
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
pyln: Migrate testing utilities and fixtures into pyln-testing #3218
Conversation
@@ -5,7 +5,7 @@ | |||
import socket | |||
import warnings | |||
|
|||
__version__ = "0.0.7.4" | |||
__version__ = "0.7.4" |
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.
Why not 0.7.3
to be in sync with the release ? 0.7.3 > 0.0.7.4
anyway
Running PYTHONPATH=`pwd`/contrib/pyln-client:`pwd`/contrib/pyln-testing:$PYTHONPATH TEST_DEBUG=1 DEVELOPER=1 VALGRIND=1 pytest tests/ -v --timeout=550 --timeout_method=thread -p no:logging -n=24 Gives me an error on the first test ( with pytest.raises(RpcError, match=r"......."):
the_call() Does not seem to catch the error. Looking forward to use |
Yep, should have marked this as WIP since it clearly has some edge-cases. For example outside of this repo we want to use the |
7edfe68
to
6eb4d1e
Compare
b334280
to
e22fd4b
Compare
e22fd4b
to
0e43862
Compare
This should not affect any consumer of the API since we just shift the actual implementation from one side to the other, and keep aliases in place so scripts don't break. We also bump the version number from 0.0.7.3 to 0.7.4 which allows us to be in sync with c-lightning itself, and remove the superfluous `0` in front.
We'll rewrite the tests to use this infrastructure in the next commit. Changelog-Added: The new `pyln-testing` package now contains the testing infrastructure so it can be reused to test against c-lighting in external projects
We use `env()` to look up configuration variables and allow them to be overridden by the environment.
We'll start relying on the in-tree version pretty soon, so we better make sure we find it.
`DEVELOPER=1` assumes that the binary has been compiled with developer set to true, which might not be the case for plugin developers. Setting this to 0 by default has no effect in c-lightning since we always at least set it in `config.vars` but may prevent some issues outside.
We were relying heavily on NodeFactory to do some magic before instantiating the Node with rpc and daemon initialized, that meant that we'd have to replace all 3 classes when customizing the node to our needs. Moving that initialization into the node itself means that the LightningNode class now can be swapped out and customized, without having to wire everything else through.
Quite a few of the things in the LightningNode class are tailored to their use in the c-lightning tests, so I decided to split those customizations out into a sub-class, and adding one more fixture that just serves the class. This allows us to override the LightningNode implementation in our own tests, while still having sane defaults for other users.
0e43862
to
169019e
Compare
I'm afraid I was not able to reproduce this issue in any of my runs. Is there anything special about your setup? Does Other than that I think this PR is pretty much ready to go ^^ |
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.
Ack 169019e
I've been planning to do this for a while now, but I didn't know how clever to get, so this is the blunt version that just moves all the pieces over while keeping the surface stable.
The goal is to split out the testing framework, including fixtures and utilities to setup and manage nodes, into its own package, so we can reuse them in other related projects. Currently we have copy-paste versions of this code in https://github.com/cdecker/lnet and https://github.com/cdecker/lightning-integration, and some people have expressed interest in setting up tests for their plugins in https://github.com/lightningd/plugins as well.
The copy in https://github.com/cdecker/lightning-integration is likely the hardest to retro-fit to use pyln-testing, since it manages multiple parametrizable implementations, but I plan to add runners for those other projects eventually as well. One step at a time :-)