Skip to content

Fix tests for Python 3.11 #7167

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

Merged
merged 5 commits into from
Jul 13, 2022
Merged

Fix tests for Python 3.11 #7167

merged 5 commits into from
Jul 13, 2022

Conversation

AdamWill
Copy link
Contributor

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

This fixes some issues in the tests with Python 3.11, and makes one change to the actual checks that is exposed by Python 3.11 - not emitting super-init-not-called for subclasses of Enum. Enum has a __init__ in Python 3.11, but it does nothing at all (just pass), and the examples in Python's own Enum docs include sample subclasses with their own __init__ which do not call Enum.__init__, so clearly it's OK/intended for you to do that. I've asked upstream why this useless Enum.__init__ was added at all.

@AdamWill
Copy link
Contributor Author

Note, this is my first time touching pylint so please review carefully! There is still a problem with the syntax_error and import_error tests, discussed somewhat at #6551 (comment) and replies - I wasn't able to come up with a sensibly upstreamable fix for that.

@coveralls
Copy link

coveralls commented Jul 11, 2022

Pull Request Test Coverage Report for Build 2666572139

  • 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 95.373%

Totals Coverage Status
Change from base Build 2665556819: 0.0%
Covered Lines: 16800
Relevant Lines: 17615

πŸ’› - Coveralls

@github-actions

This comment has been minimized.

@DanielNoord DanielNoord self-requested a review July 12, 2022 06:02
Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

Let's wait for a response on the CPython issue. I'd like to know if this is a conscious decision.

@Pierre-Sassoulas Pierre-Sassoulas added Maintenance Discussion or action around maintaining pylint or the dev workflow python 3.11 labels Jul 12, 2022
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

πŸ‘

@Pierre-Sassoulas Pierre-Sassoulas added the Blocked 🚧 Blocked by a particular issue label Jul 12, 2022
@github-actions
Copy link
Contributor

πŸ€– Effect of this PR on checked open source code: πŸ€–

Effect on astroid:
The following messages are no longer emitted:

  1. unused-argument:
    Unused argument 'context'
    https://github.com/PyCQA/astroid/blob/main/astroid/objects.py#L345
  2. unused-argument:
    Unused argument 'kwargs'
    https://github.com/PyCQA/astroid/blob/main/astroid/objects.py#L345

Effect on django:
The following messages are now emitted:

  1. too-many-instance-attributes:
    Too many instance attributes (20/11)
    https://github.com/django/django/blob/main/django/http/request.py#L47
  2. redefined-variable-type:
    Redefinition of field type from django.db.models.fields.related.OneToOneField to django.forms.widgets.Widget
    https://github.com/django/django/blob/main/django/db/models/base.py#L344

The following messages are no longer emitted:

  1. too-many-instance-attributes:
    Too many instance attributes (22/11)
    https://github.com/django/django/blob/main/django/http/request.py#L47

Effect on pandas:
The following messages are now emitted:

  1. no-member:
    Instance of 'DataFrame' has no 'one' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/frame/methods/test_matmul.py#L32
  2. no-member:
    Instance of 'DataFrame' has no 'one' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/frame/methods/test_matmul.py#L33
  3. redefined-variable-type:
    Redefinition of result type from pandas.core.indexes.datetimes.DatetimeIndex to pandas.core.indexes.multi.MultiIndex
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/multi/test_reshape.py#L126
  4. redefined-variable-type:
    Redefinition of indexer type from pandas.core.indexes.numeric.Int64Index to pandas.core.indexes.datetimes.DatetimeIndex
    https://github.com/pandas-dev/pandas/blob/main/pandas/core/algorithms.py#L1394
  5. too-many-instance-attributes:
    Too many instance attributes (33/11)
    https://github.com/pandas-dev/pandas/blob/main/pandas/core/frame.py#L468
  6. attribute-defined-outside-init:
    Attribute 'name' defined outside init
    https://github.com/pandas-dev/pandas/blob/main/pandas/core/frame.py#L9762

The following messages are no longer emitted:

  1. redefined-variable-type:
    Redefinition of result type from pandas.core.indexes.period.PeriodIndex to pandas.core.indexes.multi.MultiIndex
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/multi/test_reshape.py#L126
  2. redefined-variable-type:
    Redefinition of dti type from pandas.core.indexes.datetimes.DatetimeIndex to pandas.core.indexes.period.PeriodIndex
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/datetimes/test_misc.py#L248
  3. no-member:
    Instance of 'PeriodIndex' has no 'tz' member
    https://github.com/pandas-dev/pandas/blob/main/pandas/tests/indexes/datetimes/methods/test_shift.py#L101
  4. redefined-variable-type:
    Redefinition of indexer type from pandas.core.indexes.numeric.Int64Index to pandas.core.indexes.period.PeriodIndex
    https://github.com/pandas-dev/pandas/blob/main/pandas/core/algorithms.py#L1380
  5. too-many-instance-attributes:
    Too many instance attributes (31/11)
    https://github.com/pandas-dev/pandas/blob/main/pandas/core/frame.py#L468

Effect on sentry:
The following messages are now emitted:

  1. unused-import:
    Unused Organization imported from sentry.models
    https://github.com/getsentry/sentry/blob/master/src/sentry/testutils/fixtures.py#L7
  2. unused-import:
    Unused Organization imported from sentry.models
    https://github.com/getsentry/sentry/blob/master/src/sentry/integrations/slack/utils/channel.py#L8
  3. unused-import:
    Unused Project imported from sentry.models
    https://github.com/getsentry/sentry/blob/master/src/sentry/api/helpers/group_index/delete.py#L11
  4. unused-import:
    Unused Organization imported from sentry.models
    https://github.com/getsentry/sentry/blob/master/src/sentry/api/helpers/group_index/index.py#L15
  5. unused-import:
    Unused Project imported from sentry.models
    https://github.com/getsentry/sentry/blob/master/src/sentry/api/helpers/group_index/index.py#L15
  6. unsupported-binary-operation:
    unsupported operand type(s) for |
    https://github.com/getsentry/sentry/blob/master/src/sentry/api/endpoints/user_index.py#L27
  7. import-error:
    Unable to import 'sentry.utils.distutils.commands.base'
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_js_sdk_registry.py#L13
  8. missing-function-docstring:
    Missing function or method docstring
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_js_sdk_registry.py#L42
  9. no-self-use:
    Method could be a function
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_js_sdk_registry.py#L42
  10. too-few-public-methods:
    Too few public methods (1/2)
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_js_sdk_registry.py#L39
  11. import-error:
    Unable to import 'sentry.utils.distutils.commands.base'
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_integration_docs.py#L4
  12. missing-function-docstring:
    Missing function or method docstring
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_integration_docs.py#L10
  13. too-few-public-methods:
    Too few public methods (1/2)
    https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/build_integration_docs.py#L7

The following messages are no longer emitted:

  1. unsupported-binary-operation:
    unsupported operand type(s) for |
    https://github.com/getsentry/sentry/blob/master/src/sentry/api/endpoints/debug_files.py#L162
  2. too-many-instance-attributes:
    Too many instance attributes (20/11)
    https://github.com/getsentry/sentry/blob/master/src/sentry/mediators/param.py#L7
  3. unneeded-not:
    Consider changing "not settings.SENTRY_EVENTSTREAM == 'sentry.eventstream.kafka.KafkaEventStream'" to "settings.SENTRY_EVENTSTREAM != 'sentry.eventstream.kafka.KafkaEventStream'"
    https://github.com/getsentry/sentry/blob/master/src/sentry/runner/commands/devserver.py#L251
  4. unneeded-not:
    Consider changing "not settings.SENTRY_EVENTSTREAM == 'sentry.eventstream.kafka.KafkaEventStream'" to "settings.SENTRY_EVENTSTREAM != 'sentry.eventstream.kafka.KafkaEventStream'"
    https://github.com/getsentry/sentry/blob/master/src/sentry/runner/commands/devserver.py#L260

This comment was generated for commit 821025d

@AdamWill
Copy link
Contributor Author

I've re-factored the review changes into the original commits, and added a couple more fixes, including one for the syntax-error problem. In #6551 @DanielNoord suggested moving that test into the unit tests, but this just seems easier, at least until they change the description of this syntax error too...

@AdamWill
Copy link
Contributor Author

The CI failures look to be problems in the CI infra, not in the PR.

@DanielNoord
Copy link
Collaborator

@AdamWill Would you be willing to split out the Enum changes from this PR? I think we can merge this without it and wait for a bit on a response on the Enum changes. There have been some controversial changes to Enums in 3.11 anyway so I'm not sure they are even in their final form in beta 4 πŸ˜„

@AdamWill
Copy link
Contributor Author

@DanielNoord sure, I'll do that in a bit.

AdamWill added 4 commits July 13, 2022 11:46
This is because telnetlib is deprecated in Python 3.11. It's
hard to see exactly what this is testing - there's no great
explanation in-line and the test predates the first commit to
the git repo so we don't have a commit message to help.
telnetlib will be removed in 3.13, though, so at that point
we'll have to figure it out or drop the telnetlib part of the
test.

Signed-off-by: Adam Williamson <[email protected]>
iterable_context_py3 includes some checks that we don't emit
errors for asyncio.coroutine, but asyncio.coroutine has been
removed in Python 3.11, so we need to set a max version of
3.10 for these tests.

Signed-off-by: Adam Williamson <[email protected]>
The `binhex` module and `binascii.b2a_hqx()` function, which
were deprecated in 3.9, are removed entirely in 3.11.

Signed-off-by: Adam Williamson <[email protected]>
Python 3.11 changes the string representation of the
SyntaxError triggered by this test - it now says "expected '('"
instead of just "invalid syntax". This changes the test to use
a different error (incomplete `for` loop) which still has just
"invalid syntax" as its description in Python 3.11. This is the
same 'bad code' used in the similar `test_stdin_syntaxerror` in
the unit tests.

Signed-off-by: Adam Williamson <[email protected]>
@AdamWill
Copy link
Contributor Author

@DanielNoord done, can you clear the 'change requested'?

Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

Made a mistake in my previous comment... πŸ˜…

Thanks @AdamWill

@DanielNoord DanielNoord merged commit c6f671b into pylint-dev:main Jul 13, 2022
@AdamWill AdamWill changed the title Fix tests for Python 3.11, don't emit super-init-not-called for Enum subclasses Fix tests for Python 3.11~~, don't emit super-init-not-called for Enum subclasses~~ Jul 13, 2022
@AdamWill AdamWill changed the title Fix tests for Python 3.11~~, don't emit super-init-not-called for Enum subclasses~~ Fix tests for Python 3.11 Jul 13, 2022
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 python 3.11
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants