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

pytest: add blackbox tests for reckless #6110

Merged

Conversation

endothermicdev
Copy link
Collaborator

Because there is a lot of searching and hitting the repo server, a proxy with canned repos is used. A side-effect is the tests should be faster and more deterministic in outcome.

Changelog-None

@endothermicdev endothermicdev force-pushed the reckless-blackbox branch 2 times, most recently from 64756f6 to 59b97f6 Compare March 21, 2023 21:58
Copy link
Member

@cdecker cdecker left a comment

Choose a reason for hiding this comment

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

Feels rather heavy-handed to replicate the github web server to serve a static snapshot (blob in repo). Wouldn't it make sense to have an artificial plugin repo layed out in-tree, and work on that in-tree repo with artificial plugins instead? It'd give us more fine-grained control than dealing with real plugins that may have their very own constraints and requirements. It'd also make reproducing issues much much easier.

tests/data/rkls_api_json_lightningd_plugins Outdated Show resolved Hide resolved
tests/data/rkls_canned_git_want_lightningd_plugins Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Don't we use git to get a clone of the repository and then we mostly work on the local clone? We could skip the entire cloning step here and just have reckless work on what looks like a checked out plugins repo, so we don't need a server at all here.

Also, git repos can be served over static http, so this would already have been sufficient:

python -m http.server

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I went halfway here - the git repo is now redirected to the .git file rather than being served in multiple steps over http, which has eliminated much of the complexity. Generating the file tree and serving as json, etc., still seemed simplest in flask, so I've left it in to serve those api endpoints.

@endothermicdev endothermicdev force-pushed the reckless-blackbox branch 6 times, most recently from fdb48ee to cb3ae3d Compare April 4, 2023 20:30
@endothermicdev endothermicdev marked this pull request as draft April 4, 2023 21:45
@endothermicdev endothermicdev force-pushed the reckless-blackbox branch 2 times, most recently from e6c0090 to 04c6abb Compare April 5, 2023 21:45
They prefer Paths to be explicitly cast as strings
This should have been added earlier as @cdecker suggested, but is needed
to enable CI testing.

Changelog-Changed: Reckless - added support for networks beyond bitocoin and regtest
This will be used during CI testing in the following commit.
@endothermicdev endothermicdev force-pushed the reckless-blackbox branch 6 times, most recently from 4cf8500 to d27fa6e Compare April 7, 2023 00:20
@endothermicdev endothermicdev mentioned this pull request Apr 7, 2023
A canned lightningd/plugins is used to test against. This allows faster
and more deterministic outcomes.

Changelog-None
@endothermicdev endothermicdev marked this pull request as ready for review April 7, 2023 12:16
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack c09a3fc

@rustyrussell rustyrussell added this to the v23.05 milestone Apr 8, 2023
@rustyrussell rustyrussell merged commit 2f05062 into ElementsProject:master Apr 9, 2023
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants