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

Add regression test for #5770 #5846

Merged
merged 4 commits into from
Mar 13, 2022

Conversation

jacobtylerwalls
Copy link
Member

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Closes #5770
Refs pylint-dev/astroid#1400

@coveralls
Copy link

coveralls commented Feb 27, 2022

Pull Request Test Coverage Report for Build 1973436590

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

Totals Coverage Status
Change from base Build 1973318874: 0.0%
Covered Lines: 15029
Relevant Lines: 15984

πŸ’› - Coveralls

@jacobtylerwalls
Copy link
Member Author

jacobtylerwalls commented Feb 27, 2022

Hold on -- I have to change the variable name back to t! (Why?) I changed it to avoid a disable.
Ah, no that wasn't it.

Ah, I remember now. The regression files aren't loaded in exactly the same way that the linter loads them. So we need to find a different way to capture the test case.

@jacobtylerwalls jacobtylerwalls marked this pull request as draft February 27, 2022 17:26
@jacobtylerwalls
Copy link
Member Author

Had to enable an extension. Better to leave it here or with the tests for this extension?

@jacobtylerwalls jacobtylerwalls marked this pull request as ready for review February 27, 2022 17:44
@Pierre-Sassoulas Pierre-Sassoulas added Maintenance Discussion or action around maintaining pylint or the dev workflow Regression labels Feb 27, 2022
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.13.0 milestone Feb 27, 2022
@DanielNoord
Copy link
Collaborator

Better to test there I think. The crash probably also occurs in the extension?

@jacobtylerwalls
Copy link
Member Author

The crash probably also occurs in the extension?

The crash only occurs with the extension. I didn't notice because pylint's own pylintrc enables it.

Better to test there I think.

Sorry, just to check: you're saying move to tests/functional/ext/redefined_variable_type?

@DanielNoord
Copy link
Collaborator

DanielNoord commented Feb 27, 2022

The crash only occurs with the extension. I didn't notice because pylint's own pylintrc enables it.

Ah yeah, that's what I meant.. English πŸ˜“

Sorry, just to check: you're saying move to tests/functional/ext/redefined_variable_type?

Yes πŸ‘ In a separate file there.

@DanielNoord DanielNoord added the Needs astroid update Needs an astroid update (probably a release too) before being mergable label Mar 2, 2022
@jacobtylerwalls jacobtylerwalls merged commit 8e9a447 into pylint-dev:main Mar 13, 2022
@jacobtylerwalls jacobtylerwalls deleted the newtype-f-string branch March 13, 2022 14:21
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 Needs astroid update Needs an astroid update (probably a release too) before being mergable Regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AstroidSyntaxError when parsing NewType
4 participants