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

Fix PyreverseConfig imports in pyreverse's tests #5010

Conversation

Pierre-Sassoulas
Copy link
Member

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Fix an import error due to pytest adding all file to the pythonpath before launching tests.

@Pierre-Sassoulas Pierre-Sassoulas added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Sep 15, 2021
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.11.0 milestone Sep 15, 2021
@Pierre-Sassoulas
Copy link
Member Author

@DudeNr33 fyi, I just realized that pytest is adding all file to the pythonpath so the tests were working but not pylint or mypy :)

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1236529571

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.075%

Totals Coverage Status
Change from base Build 1235242213: 0.0%
Covered Lines: 13253
Relevant Lines: 14239

πŸ’› - Coveralls

@Pierre-Sassoulas Pierre-Sassoulas merged commit a3f3405 into pylint-dev:main Sep 15, 2021
@DanielNoord
Copy link
Collaborator

I don't actually think this fully solved the issue. My IDE can no longer detect PyreverseConfig and Pylance also complains about this. See screenshots below.
At some point mypy is going to complain about this. I would be in favour of reversing this commit.

Schermafbeelding 2021-09-15 om 16 19 43

Schermafbeelding 2021-09-15 om 16 21 48

Without this commit:
Schermafbeelding 2021-09-15 om 16 22 15

@Pierre-Sassoulas
Copy link
Member Author

How about we move this class to pylint.pyreverse? It seems it's an interface for optparse.Config that might help with typing for pylint's code too.

@DudeNr33
Copy link
Collaborator

No, not really - the "real" config class comes from the pylint.config.configuration_mixin.ConfigurationMixin, from which pylint.pyreverse.main.Run inherits from.
This is just a simplified helper class that represents the config for the tests.
However, we could move it to pylint.testutils.

@DanielNoord
Copy link
Collaborator

This is actually preventing me from running the test suite locally. I get:

../../../../.pyenv/versions/3.9.6/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/pyreverse/test_writer.py:31: in <module>
    from tests.pyreverse.conftest import PyreverseConfig
E   ModuleNotFoundError: No module named 'tests'

with tox --recreate -e py39 .

@Pierre-Sassoulas Pierre-Sassoulas deleted the fix-conftest-import-in-pyreverse branch September 15, 2021 19:54
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this pull request Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants