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

Remove primer test on black-primer #5924

Merged
merged 3 commits into from Mar 16, 2022
Merged

Remove primer test on black-primer #5924

merged 3 commits into from Mar 16, 2022

Conversation

teobouvard
Copy link
Contributor

@teobouvard teobouvard commented Mar 15, 2022

  • Add yourself to CONTRIBUTORS.txt if you are a new contributor.
  • Add a ChangeLog entry describing what your PR does.
  • If it's a new feature, or an important bug fix, add a What's New entry in
    doc/whatsnew/<current release.rst>.
  • Write a good description on what the PR does.
Type
🐛 Bug fix

Description

black-primer was removed from black repository, so primer test on this
directory has been removed to prevent the CI from failing.

See psf/black#2924

black-primer was removed from black repository, so primer test on this
directory has been removed to prevent the CI from failing.

See psf/black#2924
@teobouvard teobouvard marked this pull request as ready for review March 15, 2022 21:27
@DanielNoord
Copy link
Collaborator

@teobouvard Thanks for noticing this and coming with the quick PR! Could you remove the changelog though? The primer is only really used for us internally to check if we don't make any changes that crash on other projects. There is (currently) not really a reason for us to tell people about this in the Changelog.

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.13.0 milestone Mar 15, 2022
@Pierre-Sassoulas Pierre-Sassoulas added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Mar 15, 2022
ChangeLog Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Mar 15, 2022

Pull Request Test Coverage Report for Build 1989366817

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

Totals Coverage Status
Change from base Build 1989095492: 0.0%
Covered Lines: 15188
Relevant Lines: 16144

💛 - Coveralls

@teobouvard
Copy link
Contributor Author

teobouvard commented Mar 15, 2022

It's the second time today that the CI times out on Python 3.10 tests, any ideas why ?

@DanielNoord
Copy link
Collaborator

It's the second time today that the CI times out on Python 3.10 tests, any ideas why ?

We suspect it has something to do with the cores of the Linux runner. We test our parallel run mode while assuming the test machine has two cores available. Previously we assumed (wrongly) that 10 cores were available, which meant that the tests timed out much more often.
However, as you can see, on seemingly random PRs and distributions (although always Linux) the test still run out. If you look at the pytest output we either get to 50% of the test suite or get stuck in one of the first tests. Both come from the first tests (test_parallel) taking too long and us staying stuck there or reaching 50% before eventually timing out.

We have debated whether we should remove the timeout for the tests. Personally I think it is still fine. Although annoying, the current situation is somewhat better than having runners being occupied for over an hour doing a test suite that is taking unnecessarily long. Also, all maintainers know that this is an issue and now not to look into it too much when it occurs on a specific PR.

In the long term we would like to get a solution here, but after many attempts I'm really stuck for any solutions. The documentation from Github says that all Linux runners should have two cores and all tests pass fine locally, but for some reason on certain runners there are issues. I haven't found any discussion online as of yet that could allude to what is going on here.

@teobouvard
Copy link
Contributor Author

Although annoying, the current situation is somewhat better than having runners being occupied for over an hour doing a test suite that is taking unnecessarily long.

True, and since this is not really an issue that can be debugged easily, I think keeping timeouts is reasonable. Thanks for the information.

@DanielNoord DanielNoord merged commit 65f65e9 into pylint-dev:main Mar 16, 2022
@teobouvard teobouvard deleted the remove-black-primer branch March 16, 2022 07:59
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