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

FIXES REGRESSION - Accommodates black.files.find_pyproject_toml returning None #1956

Merged
merged 1 commit into from
Jan 13, 2022
Merged

Conversation

posita
Copy link
Contributor

@posita posita commented Jan 12, 2022

PR Summary

Prior to this, the return value of black.files.find_pyproject_toml was passed directly
to os.path.exists. This was fine where black could find a pyproject.toml file.
However, when one cannot be located, find_pyproject_toml returns None, which results
in an uncaught exception from os.path.exists. This adds an additional check for when
find_project_toml returns None.

Fixes #1955 (introduced by #1950).

PR checklist

Please make sure that the following things have been addressed (and check the relevant checkboxes):

  • Commits respect our guidelines
  • Tests are passing properly (see here on how to run Elpy's tests)
    I have only run Python-based tests, which were not passing before I submitted this PR. I have verified that this PR introduces no new Python test failures. In fact it fixes three of the four existing failures in my local testing. (See this comment below for detail.)

Prior to this, the return value of `black.files.find_pyproject_toml` was passed directly
to `os.path.exists`. This was fine where `black` could find a `pyproject.toml` file.
However, when one cannot be located, `find_pyproject_toml` returns `None`, which results
in an uncaught exception from `os.path.exists`. This adds an additional check for when
`find_project_toml` returns `None`.

Fixes #1955 (introduced by #1950).
@posita
Copy link
Contributor Author

posita commented Jan 12, 2022

@jeremyheiler, do you have any experience with running tests? I'm not set up to do that. If you are, any chance I can convince you to run them for this PR and report back?

@posita posita mentioned this pull request Jan 12, 2022
2 tasks
@posita posita changed the title FIXES REGRESSION - Accommodates find_pyproject_toml returning None FIXES REGRESSION - Accommodates black.files.find_pyproject_toml returning None Jan 12, 2022
@coveralls
Copy link

Coverage Status

Coverage remained the same at 82.863% when pulling 5d3d329 on posita:posita/fix-1955 into 9e4382f on jorgenschaefer:master.

@posita
Copy link
Contributor Author

posita commented Jan 13, 2022

I can verify that this PR fixes several prior failing tests. When I run tests locally on master I get the following errors:

======================================================================
ERROR: test_fix_code (elpy.tests.test_black.BLACKTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/matt/Documents/dev/3p/elpy/elpy/tests/test_black.py", line 37, in test_fix_code
    self._assert_format(src, expected)
  File "/Users/matt/Documents/dev/3p/elpy/elpy/tests/test_black.py", line 48, in _assert_format
    new_block = blackutil.fix_code(src, os.getcwd())
  File "/Users/matt/Documents/dev/3p/elpy/elpy/blackutil.py", line 58, in fix_code
    if toml is not None and os.path.exists(pyproject_path):
  File "/Library/Frameworks/MacPorts-20211130/Python.framework/Versions/3.9/lib/python3.9/genericpath.py", line 19, in exists
    os.stat(path)
TypeError: stat: path should be string, bytes, os.PathLike or integer, not NoneType

======================================================================
ERROR: test_fix_code_should_throw_error_for_invalid_code (elpy.tests.test_black.BLACKTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/matt/Documents/dev/3p/elpy/elpy/tests/test_black.py", line 21, in test_fix_code_should_throw_error_for_invalid_code
    self.assertRaises(Fault, blackutil.fix_code, src, os.getcwd())
  File "/Library/Frameworks/MacPorts-20211130/Python.framework/Versions/3.9/lib/python3.9/unittest/case.py", line 739, in assertRaises
    return context.handle('assertRaises', args, kwargs)
  File "/Library/Frameworks/MacPorts-20211130/Python.framework/Versions/3.9/lib/python3.9/unittest/case.py", line 201, in handle
    callable_obj(*args, **kwargs)
  File "/Users/matt/Documents/dev/3p/elpy/elpy/blackutil.py", line 58, in fix_code
    if toml is not None and os.path.exists(pyproject_path):
  File "/Library/Frameworks/MacPorts-20211130/Python.framework/Versions/3.9/lib/python3.9/genericpath.py", line 19, in exists
    os.stat(path)
TypeError: stat: path should be string, bytes, os.PathLike or integer, not NoneType

======================================================================
ERROR: test_perfect_code (elpy.tests.test_black.BLACKTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/matt/Documents/dev/3p/elpy/elpy/tests/test_black.py", line 45, in test_perfect_code
    self._assert_format(src, expected)
  File "/Users/matt/Documents/dev/3p/elpy/elpy/tests/test_black.py", line 48, in _assert_format
    new_block = blackutil.fix_code(src, os.getcwd())
  File "/Users/matt/Documents/dev/3p/elpy/elpy/blackutil.py", line 58, in fix_code
    if toml is not None and os.path.exists(pyproject_path):
  File "/Library/Frameworks/MacPorts-20211130/Python.framework/Versions/3.9/lib/python3.9/genericpath.py", line 19, in exists
    os.stat(path)
TypeError: stat: path should be string, bytes, os.PathLike or integer, not NoneType

======================================================================
FAIL: test_should_get_docstring (elpy.tests.test_jedibackend.TestRPCGetDocstring)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/matt/Documents/dev/3p/elpy/elpy/tests/support.py", line 796, in test_should_get_docstring
    self.check_docstring(docstring)
  File "/Users/matt/Documents/dev/3p/elpy/elpy/tests/test_jedibackend.py", line 79, in check_docstring
    self.assertIsNotNone(match)
AssertionError: unexpectedly None

After applying this PR, I only get the error on test_should_get_docstring, which was also broken on master and appears unrelated.

I do not know why those tests are not failing in CI.

@posita
Copy link
Contributor Author

posita commented Jan 13, 2022

@jorgenschaefer, I think this is ready for review/merge. The remaining test failure appears unrelated to this issue.

@gopar gopar merged commit edea332 into jorgenschaefer:master Jan 13, 2022
@gopar
Copy link
Collaborator

gopar commented Jan 13, 2022

LGTM. Merging.
@posita Please don't ping Jorgen, he's taking a break from maintaining elpy :)

@posita
Copy link
Contributor Author

posita commented Jan 13, 2022

LGTM. Merging. @posita Please don't ping Jorgen, he's taking a break from maintaining elpy :)

Oh shoot! Thanks for the merge, and apologies for misdirected at-mention! 😬

@posita posita deleted the posita/fix-1955 branch January 19, 2022 03:53
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.

elpy-black-fix-code RPC triggers nil error
3 participants