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

MAINT: pin pytest<7 in pypy3 CI #1262

Merged
merged 1 commit into from Feb 7, 2022
Merged

Conversation

lesteve
Copy link
Member

@lesteve lesteve commented Feb 7, 2022

similar to #1261 but for pypy

Also tweak the comments, I don't think this causes freezes but only test errors because there are additional PytestRemovedIn8Warning (but maybe I missed something ...)

@codecov
Copy link

codecov bot commented Feb 7, 2022

Codecov Report

Merging #1262 (31e7f53) into master (732923a) will increase coverage by 0.15%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1262      +/-   ##
==========================================
+ Coverage   88.21%   88.37%   +0.15%     
==========================================
  Files          47       47              
  Lines        7045     7045              
==========================================
+ Hits         6215     6226      +11     
+ Misses        830      819      -11     
Impacted Files Coverage Δ
joblib/hashing.py 91.22% <0.00%> (+1.75%) ⬆️
joblib/_parallel_backends.py 91.91% <0.00%> (+3.30%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 732923a...31e7f53. Read the comment docs.

@lesteve lesteve merged commit 88fef12 into joblib:master Feb 7, 2022
@lesteve lesteve deleted the pytest6-in-pypy branch February 7, 2022 14:22
@tomMoral
Copy link
Contributor

tomMoral commented Feb 7, 2022

It seems that the error is on our side no?
Following this discussion, it seems that we should simply switch to warning.catch_warning(record=True) instead of pytest.warns(None).

I will do a PR for this.

@ogrisel
Copy link
Contributor

ogrisel commented Feb 7, 2022

I started to do a PR but supporting both pytest 6 and pytest 7 at the same time without error in warning tests is complicated...

@tomMoral
Copy link
Contributor

tomMoral commented Feb 7, 2022

Even with the change proposed in the PR by scipy?

https://github.com/scipy/scipy/pull/15192/files

@ogrisel
Copy link
Contributor

ogrisel commented Feb 7, 2022

I had tried something more complex, then started to use this approach but then got a stalled pytest 7 problem after the test finish and I abandoned.

@ogrisel
Copy link
Contributor

ogrisel commented Feb 7, 2022

I did not investigate what was causing the staleness.

@tomMoral
Copy link
Contributor

tomMoral commented Feb 7, 2022

I tried to make a PR in #1264, if it stalls, I will try to investigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants