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

test(conftest): support @pytest.mark.bashcomp(required_cmd=True) #547

Merged
merged 1 commit into from
Jun 30, 2021

Conversation

akinomyoga
Copy link
Collaborator

Separate commit 53060e3 in #546

@scop
Copy link
Owner

scop commented Jun 30, 2021

Needs to be run through black, feel free to merge after fixing. Highly recommended to use pre-commit locally to avoid roundtrips like this.

@akinomyoga akinomyoga force-pushed the support-bashcmop-require_cmd branch from 53060e3 to 01959d2 Compare June 30, 2021 09:47
@akinomyoga
Copy link
Collaborator Author

Ah, OK. I've fixed it. I thought I have run pre-commit previously for #544, but it seems I have skipped it because #544 is still a draft PR.

@akinomyoga
Copy link
Collaborator Author

The test has passed, so I merge this PR.

@akinomyoga akinomyoga merged commit 33df79c into scop:master Jun 30, 2021
@akinomyoga akinomyoga deleted the support-bashcmop-require_cmd branch June 30, 2021 10:31
@scop
Copy link
Owner

scop commented Jun 30, 2021

I thought I have run pre-commit previously for #544, but it seems I have skipped it because #544 is still a draft PR.

I suggest installing pre-commit locally with pre-commit install so there is never need to remember to run it, it will just happen automatically when one tries to commit. git commit -n can be used to bypass it if it causes problems e.g. with one's workflow (intermediate WIP commits that should not be linted yet or so).

@akinomyoga
Copy link
Collaborator Author

Yeah, I once installed the pre-commit hook after you have suggested it to me before. But after that, pre-commit interfered with git rebase -i and caused troubles. Since I frequently do git rebase, I have uninstalled it and decided to run it manually.

@scop
Copy link
Owner

scop commented Jul 6, 2021

FWIW I use pre-commit and rebase with and without -i a lot, and I don't think I've ever run into problems with that combo.

@akinomyoga
Copy link
Collaborator Author

akinomyoga commented Jul 6, 2021

Oh, yeah. I didn't remember the exact situation that I met with git rebase, so I had actually re-installed the pre-commit hook again after my previous comment. Then, I remembered that it was related to misconfigured mypy tests. In my environment, mypy produces the following error, which prevent me of making commits touching any *.py files:

mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

No virtual environments found

When I first installed the hook, I first faced with this issue in git rebase where I tried to modify an older commit and there are some newer commits touching *.py after the modified commit. Since you have mentioned that there is the same issue in your environment with mypy in #526 (comment), I just gave up to try to fix this mypy issue. Do you have any idea on how to fix this problem?

@scop
Copy link
Owner

scop commented Jul 7, 2021

Our dev env setup instructions are inadequate, indeed a python virtualenv must be set up in the bash-completion working dir, then the above should work. Basically the commands in this run section need to be run locally, or replacing the first two with a functionally equivalent pyenv and pyenv-virtualenv setup (that's what I'm using), venv-run then picks the env up and things should work in principle. The other mypy related issue you mentioned is something different, this doesn't fix that one, but I don't think I'm hitting it every time.

@akinomyoga
Copy link
Collaborator Author

OK thank you for your instruction! It now works correctly.

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

2 participants