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

Click version 7.1 breaks test and verdi-autodocs pre-commit hook #3838

Closed
csadorf opened this issue Mar 9, 2020 · 3 comments · Fixed by #3921
Closed

Click version 7.1 breaks test and verdi-autodocs pre-commit hook #3838

csadorf opened this issue Mar 9, 2020 · 3 comments · Fixed by #3921
Assignees
Milestone

Comments

@csadorf
Copy link
Contributor

csadorf commented Mar 9, 2020

The click version 7.1 (released a few hours ago) breaks test and verdi-autodocs pre-commit hook.

As an immediate measure, I recommend to pin the Click version to 7.0 (#3837 ).

@sphuber
Copy link
Contributor

sphuber commented Apr 11, 2020

Do you remember what the problem was exactly? I tried to unpin and verdi-autodocs ran just fine with the new version of click. It just has some other setting regarding to the help string formatting, so it changed some of the generated auto docs.

@greschd
Copy link
Member

greschd commented Apr 11, 2020

Might be unrelated, but click version 7.1 had broken its error messages (they went to stdout instead of stderr). The fixed 7.1.1 was released just a few hours after, but not before the issue mentioned above.

If I remember correctly I did try to run the tests again with 7.1.1 and it was still broken, but I didn't test that thoroughly.

@sphuber
Copy link
Contributor

sphuber commented Apr 12, 2020

The problem is due to this PR of click. They wrapped the editor command in shlex_quote. Since in our unit testing we pass arguments to the editor command to make it non-interactive, this is now interpreted as a single executable by bash which it then fails to find. We might have to find another way to simulate the interactivity of these tests without abusing the os.environ['EDITOR'] variable.

Edit: and we are also affected by this issue.

@sphuber sphuber self-assigned this Apr 12, 2020
@sphuber sphuber added priority/quality-of-life would simplify development topic/testing labels Apr 12, 2020
@sphuber sphuber added this to the v1.2.0 milestone Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants