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

Weekly cleanups #4417

Merged
merged 2 commits into from
Mar 11, 2021
Merged

Weekly cleanups #4417

merged 2 commits into from
Mar 11, 2021

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Mar 9, 2021

  • We improve the detection of pytest to prefer python3 -m pytest
    over the other executables since it is checked for just
    earlier. Also pytest-3 must not be given special treatment since
    it refers to version 3 of pytest (ancient nowadays) not the python3
    version of pytest.

  • Found a regression in pyln: the spent parameter to the
    listfunds call would be always included, which would lead
    c-lightning versions 0.9.2 and older to complain about an unknown
    parameter. Always set these to None so pyln can remove them from
    the call, I didn't catch his in my review.

@cdecker cdecker marked this pull request as draft March 9, 2021 09:28
We were looking for `python` in the `pytest --version` output
The semantics don't change, since `lightningd` will use false as
default as well, however setting it to something other than `None`
causes the RPC library to include the parameter in the query, and
since the parameter was introduced only in 0.9.3 and pyln may be used
with older versions this then results in an error about an unknown
parameter.
Setting this to `None` makes sure pyln filters out the argument before
calling.

Changelog-Fixed: pyln: Fixed an error when calling `listfunds` with an older c-lightning version causing an error about an unknown `spent` parameter
@cdecker cdecker marked this pull request as ready for review March 9, 2021 09:29
@cdecker cdecker added this to the v0.9.4 milestone Mar 9, 2021
Copy link
Collaborator

@niftynei niftynei left a comment

Choose a reason for hiding this comment

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

ACK adbe20d

# Since we just checked that we have python3 we give that one the
# most priority and then fall back to some common aliases.
PYTEST_BINS="python3 -m pytest,pytest,py.test,pytest3,pytest-3"
IFS=','
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to revert/unset IFS after this?

@rustyrussell rustyrussell merged commit 1e6626f into ElementsProject:master Mar 11, 2021
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.

3 participants