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

Pin pytest to resolve issue with pytest-benchmark #7674

Merged
merged 1 commit into from
Oct 25, 2022

Conversation

cdce8p
Copy link
Member

@cdce8p cdce8p commented Oct 25, 2022

Description

Fix issue with pytest v7.2.0 and pytest-benchmark discovered in #7671
Ref: ionelmc/pytest-benchmark#226

@cdce8p cdce8p added Maintenance Discussion or action around maintaining pylint or the dev workflow Skip news 🔇 This change does not require a changelog entry labels Oct 25, 2022
@cdce8p cdce8p added this to the 2.15.6 milestone Oct 25, 2022
@Pierre-Sassoulas Pierre-Sassoulas enabled auto-merge (squash) October 25, 2022 19:24
@Pierre-Sassoulas Pierre-Sassoulas merged commit 3e46dc7 into pylint-dev:main Oct 25, 2022
@cdce8p
Copy link
Member Author

cdce8p commented Oct 25, 2022

Seems like we shouldn't use auto-merge 🤔
It merged the PR before the Windows tests were run.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3323666920

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

Totals Coverage Status
Change from base Build 3320301213: 0.0%
Covered Lines: 17178
Relevant Lines: 18014

💛 - Coveralls

@cdce8p cdce8p deleted the fix-pytest branch October 25, 2022 19:42
@Pierre-Sassoulas
Copy link
Member

Seems like we shouldn't use auto-merge

Right, I forgot. We could add more required tests but it can be annoying when we want to merge fast.

@github-actions
Copy link
Contributor

🤖 Effect of this PR on checked open source code: 🤖

Effect on pytest:
The following messages are now emitted:

  1. inconsistent-return-statements:
    Either all return statements in a function should return an expression, or none of them should.
    https://github.com/pytest-dev/pytest/blob/bbe7cbae4aa24f4cb65230704b870c5dcf40781e/src/_pytest/pytester.py#L301
  2. no-name-in-module:
    No name 'NOSE_SUPPORT_METHOD' in module '_pytest.deprecated'
    https://github.com/pytest-dev/pytest/blob/bbe7cbae4aa24f4cb65230704b870c5dcf40781e/src/_pytest/python.py#L62
  3. no-name-in-module:
    No name 'PytestReturnNotNoneWarning' in module '_pytest.warning_types'
    https://github.com/pytest-dev/pytest/blob/bbe7cbae4aa24f4cb65230704b870c5dcf40781e/src/_pytest/python.py#L81
  4. inconsistent-return-statements:
    Either all return statements in a function should return an expression, or none of them should.
    https://github.com/pytest-dev/pytest/blob/bbe7cbae4aa24f4cb65230704b870c5dcf40781e/src/_pytest/python.py#L1085
  5. inconsistent-return-statements:
    Either all return statements in a function should return an expression, or none of them should.
    https://github.com/pytest-dev/pytest/blob/bbe7cbae4aa24f4cb65230704b870c5dcf40781e/src/_pytest/fixtures.py#L130
  6. inconsistent-return-statements:
    Either all return statements in a function should return an expression, or none of them should.
    https://github.com/pytest-dev/pytest/blob/bbe7cbae4aa24f4cb65230704b870c5dcf40781e/src/_pytest/python_api.py#L798
  7. no-name-in-module:
    No name 'NOSE_SUPPORT' in module '_pytest.deprecated'
    https://github.com/pytest-dev/pytest/blob/bbe7cbae4aa24f4cb65230704b870c5dcf40781e/src/_pytest/nose.py#L5
  8. no-name-in-module:
    No name 'warn_explicit_for' in module '_pytest.warning_types'
    https://github.com/pytest-dev/pytest/blob/bbe7cbae4aa24f4cb65230704b870c5dcf40781e/src/_pytest/config/__init__.py#L62
  9. no-member:
    Module '_pytest.deprecated' has no 'HOOK_LEGACY_MARKING' member
    https://github.com/pytest-dev/pytest/blob/bbe7cbae4aa24f4cb65230704b870c5dcf40781e/src/_pytest/config/__init__.py#L369

This comment was generated for commit a2f3c7a

@cdce8p
Copy link
Member Author

cdce8p commented Oct 26, 2022

Seems like we shouldn't use auto-merge

Right, I forgot. We could add more required tests but it can be annoying when we want to merge fast.

I've added run / 3.11 / Linux and run / 3.11 / Windows. That way we know that at least some things are working before an auto-merge.

@cdce8p cdce8p added the Needs backport Needs to be cherry-picked on the current patch version by a pylint's maintainer label Oct 26, 2022
@Pierre-Sassoulas
Copy link
Member

run / 3.11 / Windows

The problem was that this is one of the slower tests (only primer is longer). Ho well, we're just going to wait a little longer before merging typo-MR 🤷

@Pierre-Sassoulas
Copy link
Member

Beside we'll be able to enable auto-merge which was the point. Now that I think about it we could add primers and always be able to use auto-merge ?

@cdce8p
Copy link
Member Author

cdce8p commented Oct 26, 2022

Beside we'll be able to enable auto-merge which was the point. Now that I think about it we could add primers and always be able to use auto-merge ?

The primer tests will / should never fail. I don't think that will work as intended. Even if it's a bit more work, doing the merge manual is the best option for us I think.

@Pierre-Sassoulas
Copy link
Member

The issue with required job like this is that for documentation merge request where we don't want to run all the tests we can't merge them if tests are required.

@cdce8p
Copy link
Member Author

cdce8p commented Oct 26, 2022

We could always modify the branch protection rules and remove the Do not allow bypassing the above settings option. Something I would recommend anyway as it's limited to admins and even we would need to click a checkbox before we can merge something without all checks.

@Pierre-Sassoulas
Copy link
Member

Agreed and done !

@Pierre-Sassoulas Pierre-Sassoulas added Backported and removed Needs backport Needs to be cherry-picked on the current patch version by a pylint's maintainer labels Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported Maintenance Discussion or action around maintaining pylint or the dev workflow Skip news 🔇 This change does not require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants