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

Run PyPy 3.8 and 3.9 in CI #6406

Merged
merged 3 commits into from
May 3, 2022
Merged

Conversation

DanielNoord
Copy link
Collaborator

  • Add yourself to CONTRIBUTORS if you are a new contributor.
  • Write a good description on what the PR does.

Type of Changes

Type
βœ“ πŸ”¨ Refactoring

Description

Blocked by pylint-dev/astroid#1520.

@DanielNoord DanielNoord added 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 labels Apr 20, 2022
@DanielNoord DanielNoord added this to the 2.14.0 milestone Apr 20, 2022
@DanielNoord DanielNoord mentioned this pull request Apr 20, 2022
15 tasks
@coveralls
Copy link

coveralls commented Apr 20, 2022

Pull Request Test Coverage Report for Build 2260566085

  • 4 of 4 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.0006%) to 95.168%

Totals Coverage Status
Change from base Build 2259008088: 0.0006%
Covered Lines: 15834
Relevant Lines: 16638

πŸ’› - Coveralls

@Pierre-Sassoulas
Copy link
Member

Should we move that to 2.15 ?

@DanielNoord
Copy link
Collaborator Author

Yes!

@DanielNoord DanielNoord modified the milestones: 2.14.0, 2.15.0 Apr 20, 2022
pylint/testutils/checker_test_case.py Outdated Show resolved Hide resolved
pylint/testutils/checker_test_case.py Outdated Show resolved Hide resolved
@cdce8p
Copy link
Member

cdce8p commented Apr 22, 2022

Unfortunately, there is still one failing test even with the latest astroid changes.

__________________ test_functional[unspecified_encoding_py38] __________________

self = <pylint.testutils.lint_module_test.LintModuleTest object at 0x000000000bbc6aa0>

    def runTest(self) -> None:
>       self._runTest()
E       AssertionError: Wrong results for file "unspecified_encoding_py38":
E       
E       Expected in testdata:
E         13: unspecified-encoding
E         14: unspecified-encoding
E         15: unspecified-encoding
E         16: unspecified-encoding
E         17: unspecified-encoding
E         26: unspecified-encoding
E         29: unspecified-encoding
E         33: unspecified-encoding
E         38: unspecified-encoding
E         39: unspecified-encoding
E         40: unspecified-encoding
E         41: unspecified-encoding
E         50: unspecified-encoding
E         53: unspecified-encoding
E         57: unspecified-encoding
E        149: unspecified-encoding
E        161: bad-open-mode
E        161: unspecified-encoding
E        164: bad-open-mode
E        164: unspecified-encoding
E 

@DanielNoord
Copy link
Collaborator Author

The open issue is due to astroid on PyPy failing the following test:

def test_open_pypy() -> None:
    """Regression test for inference of open() call.

    Previously we failed to infer this on PyPy.
    """
    node: nodes.Call = builder.extract_node("""open("foo.py", "w", encoding="utf-8")""")
    inferred = node.func.inferred()
    assert len(inferred) == 1
    assert isinstance(inferred[0], nodes.FunctionDef)
    assert inferred[0].name == "open"

I think this is due to how builtins are defined but I am by no means a PyPy expert..

@DanielNoord
Copy link
Collaborator Author

I have published a fix for the inference of open to astroid. Apparently this was a very old issue, which now finally has a solution :)

@DanielNoord DanielNoord force-pushed the pypy branch 2 times, most recently from 6e94d0f to 89d7129 Compare May 2, 2022 14:47
@DanielNoord DanielNoord force-pushed the pypy branch 2 times, most recently from c1a9e07 to 929e6ae Compare May 2, 2022 20:47
@DanielNoord DanielNoord changed the title Run PyPy 3.8 in CI Run PyPy 3.8 and 3.9 in CI May 2, 2022
@DanielNoord DanielNoord removed the Needs astroid update Needs an astroid update (probably a release too) before being mergable label May 2, 2022
@DanielNoord DanielNoord modified the milestones: 2.15.0, 2.14.0 May 2, 2022
@DanielNoord DanielNoord marked this pull request as ready for review May 2, 2022 21:38
@DanielNoord
Copy link
Collaborator Author

Actually was able to run on 3.9 as well with some minor changes πŸ˜„

@DanielNoord
Copy link
Collaborator Author

@Pierre-Sassoulas Gentle ping, I feel you might have missed this. Never mind if you just haven't gotten to it!

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.

Thank you for the reminder, I did miss it :) Looks great !

@Pierre-Sassoulas Pierre-Sassoulas merged commit 003336a into pylint-dev:main May 3, 2022
@DanielNoord DanielNoord deleted the pypy branch May 3, 2022 21:46
@cdce8p
Copy link
Member

cdce8p commented May 3, 2022

Thanks for working on this @DanielNoord! Saw you added PyPy 3.9 already. AFAIK we do have one more issue with it in astroid. Last I checked it was something with detecting the builtins module. Not sure if that has already been fixed in the mean time.

@DanielNoord
Copy link
Collaborator Author

Thanks for working on this @DanielNoord! Saw you added PyPy 3.9 already. AFAIK we do have one more issue with it in astroid. Last I checked it was something with detecting the builtins module. Not sure if that has already been fixed in the mean time.

I don't think so, but with my new experience with builtins and io I might be able to tackle it! I'll try and have a look at some point.

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