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

Generic plugin support #491

Merged
merged 2 commits into from
Feb 9, 2024

Conversation

daywalker90
Copy link
Contributor

I've thought more about #487 and since CLN supports basically any language for plugins i propose to take this generic approach instead.
Non-python plugins put their requirements.txt and setup.sh in the tests folder (the one we already search for executing tests). The bash script will allow python developer to setup the plugin environment as they see fit.

Example setup.sh from my summars plugin and then a simple switch in my utils.py for local dev and CI

@chrisguida
Copy link
Collaborator

Neat! So we just require plugins to provide a release binary via a setup.sh script, and then we use that for the pytests?

@chrisguida
Copy link
Collaborator

If I run the CI now, will it test anything?

@daywalker90
Copy link
Contributor Author

daywalker90 commented Feb 7, 2024

No because there is no non-python plugin that has passing tests in the repo merged atm. But i tested it on my fork (where i merged #485 and #486 and a special branch of #454 ) here:
https://github.com/daywalker90/plugins/actions/runs/7820798111/job/21336309829
EDIT: dont know why some tests are timing out on wait_for_channel, investigating
EDIT2: Seems like its a problem with cln master branch

@daywalker90
Copy link
Contributor Author

daywalker90 commented Feb 7, 2024

Neat! So we just require plugins to provide a release binary via a setup.sh script, and then we use that for the pytests?

yes, the plugin author uses it in their tests, you don't have to do anything, see my utils.py i linked

@daywalker90
Copy link
Contributor Author

I was in need of getting the python bin that is using the specific test env into the setup.sh. I think there could be some other use cases for this, so i added the env variable TEST_DIR.

@daywalker90
Copy link
Contributor Author

rebased

@chrisguida
Copy link
Collaborator

ACK fb1637d

@chrisguida
Copy link
Collaborator

@Mergifyio queue

Copy link

mergify bot commented Feb 9, 2024

queue

🛑 The pull request has been removed from the queue default

Pull request #491 has been dequeued due to failing checks or checks timeout.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@chrisguida
Copy link
Collaborator

@Mergifyio refresh

Copy link

mergify bot commented Feb 9, 2024

refresh

✅ Pull request refreshed

@chrisguida
Copy link
Collaborator

@Mergifyio queue

Copy link

mergify bot commented Feb 9, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at cb3145d

@mergify mergify bot merged commit cb3145d into lightningd:master Feb 9, 2024
13 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.

2 participants