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

Also test on PyPy 3.7 #5853

Merged
merged 1 commit into from Mar 7, 2022
Merged

Also test on PyPy 3.7 #5853

merged 1 commit into from Mar 7, 2022

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

See #5849 (comment).

@DanielNoord DanielNoord added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Mar 1, 2022
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.13.0 milestone Mar 1, 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.

I'm ok with fixing the issue with the line/col either in 2.13 or 2.14 :)

@@ -476,7 +476,7 @@ jobs:
with:
path: venv
key:
${{ runner.os }}-${{ steps.python.outputs.python-version }}-${{
${{ runner.os }}-${{ matrix.python-version }}-${{
Copy link
Member

@cdce8p cdce8p Mar 1, 2022

Choose a reason for hiding this comment

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

This shouldn't be necessary. The Set up Python version will output ${{ matrix.python-version }}. As far as I can tell, the tests didn't crash.

https://github.com/PyCQA/pylint/runs/5376859270?check_suite_focus=true

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, this doesn't work as well.

Please see the version of pytest in the pypy-3.7 run. That's the 3.6 version. You can see they are using the same cache in the Restore Python virtual environment job. The key is the same.

However, my fix also doesn't work. I guess something weird is going on with python-version and PyPy? For the cpython jobs the keys do work..

Copy link
Member

Choose a reason for hiding this comment

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

This pretty much looks like a bug in the setup-python actions. If I had to guess a result of actions/setup-python#342.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I opened actions/setup-python#345.

I'll draft this until I get a response.

Copy link
Member

Choose a reason for hiding this comment

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

I've included the fix in #5868
Not sure when the bug will be addressed. We can always revert it, once a new version is released.

After #5868 is done, we should be able to move forward here.

@DanielNoord
Copy link
Collaborator Author

There is an issue with the current caching on PyPy. For some reason steps.python.outputs.python-version is empty on the PyPy jobs. I'm not sure why.
You can see in the first commit checks that pypy-3.7 actually uses 3.6 for pytest. They share the same cache. For 3.6 and 3.7 this isn't a problem, but it will be when we add 3.8 and it was also the causes of the issues I had in #5849.

I could do a separate PR for the fix, but I think it should be fine to squash here, or to add two commits and rebase here.


For the future: we should remove 3.6 when we officially drop support of 3.6. After that we should look into testing PyPy on 3.8 as well and seeing what that requires from us.

@coveralls
Copy link

coveralls commented Mar 1, 2022

Pull Request Test Coverage Report for Build 1916864181

  • 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.997%

Totals Coverage Status
Change from base Build 1916776999: 0.0%
Covered Lines: 14937
Relevant Lines: 15891

πŸ’› - Coveralls

@DanielNoord DanielNoord marked this pull request as draft March 1, 2022 15:00
@DanielNoord DanielNoord removed this from the 2.13.0 milestone Mar 4, 2022
Copy link
Member

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

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

I've rebased the PR to only include the addition of pypy-3.7.
Don't know if you wanted to do anything else, so will leave it as draft for now. Feel free to merge it though if it's finished.

@cdce8p cdce8p added this to the 2.13.0 milestone Mar 7, 2022
@DanielNoord
Copy link
Collaborator Author

Thanks @cdce8p, I'm going to merge this!

@DanielNoord DanielNoord marked this pull request as ready for review March 7, 2022 21:52
@DanielNoord DanielNoord merged commit 4649153 into pylint-dev:main Mar 7, 2022
@DanielNoord DanielNoord deleted the pypy branch March 7, 2022 21:52
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.

None yet

4 participants