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

Fixing tests failing on MacOS #178

Merged
merged 3 commits into from
Dec 9, 2022
Merged

Fixing tests failing on MacOS #178

merged 3 commits into from
Dec 9, 2022

Conversation

andrea-pasquale
Copy link
Contributor

@andrea-pasquale andrea-pasquale commented Dec 9, 2022

Closes #177 by removing pre-commit and black from dev deps.

Currently tests are failing because there is an issue with the latest release of black on MacOS.

After talking to @alecandido we agreed that actually both black and pre-commit are not needed inside pyproject.toml. In fact pre-commit is not needed inside the environment given that the user could commit outside the environment generated by poetry. The same thing is true for black given that we are using it only with pre-commit.

@scarrazza let me know if you agree.

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.

@codecov
Copy link

codecov bot commented Dec 9, 2022

Codecov Report

Merging #178 (6c83fcd) into main (3da0feb) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #178   +/-   ##
=======================================
  Coverage   27.28%   27.28%           
=======================================
  Files          17       17           
  Lines         722      722           
=======================================
  Hits          197      197           
  Misses        525      525           
Flag Coverage Δ
unittests 27.28% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@alecandido
Copy link
Member

alecandido commented Dec 9, 2022

Just to add a few more details for a future reader:

  • concerning pre-commit, you want to commit outside the environment: any IDE will be outside the environment, but also in a terminal you should try to use poetry shell as little as possible, prefer poetry run (that of course only works for single commands, but at least you always know what you are using), and you do not want to commit inside the Poetry shell (also because it is by design an isolated environment, and you can not use any Python CLI installed system or user-wide) - for the CI it is just a useless dependency
  • concerning black, instead it's even harmful: if pre-commit and Poetry happen to have two different versions of Black, if you run inside Poetry you will also enter a loop, in which one modify the code one way and the other revert it

@scarrazza scarrazza added the bug Something isn't working label Dec 9, 2022
@scarrazza scarrazza added this to the First release milestone Dec 9, 2022
Copy link
Member

@scarrazza scarrazza left a comment

Choose a reason for hiding this comment

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

Thanks, I fully agree.

@scarrazza scarrazza merged commit f539229 into main Dec 9, 2022
@scarrazza scarrazza deleted the downgrade_black branch December 9, 2022 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Tests failing on mac os
3 participants