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

[3.8] Make the 3.10 related xfails non-strict #7178

Merged
merged 1 commit into from
Jul 7, 2023

Conversation

hroncok
Copy link

@hroncok hroncok commented Jan 20, 2023

What do these changes do?

We see xpasses in Fedora with Python 3.11.1 or 3.10.9:

=================================== FAILURES ===================================
__________________________ test_default_loop[pyloop] ___________________________
[XPASS(strict)] No idea why ClientRequest() is constructed out of loop but it calls `asyncio.get_event_loop()`
____________________ TestStreamReader.test_ctor_global_loop ____________________
[XPASS(strict)] No idea why ClientRequest() is constructed out of loop but it calls `asyncio.get_event_loop()`
__________________________ test_set_loop_default_loop __________________________
[XPASS(strict)] No idea why _set_loop() is constructed out of loop but it calls `asyncio.get_event_loop()`

Are there changes in behavior for the user?

No.

Related issue number

No.

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • [-] Documentation reflects the changes
  • [-] If you provide code modification, please add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> for example (588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the pr
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

@hroncok
Copy link
Author

hroncok commented Jan 20, 2023

Opening as a draft without the changelog things for now. I am not sure this is the bast approach to solving our issue.

@musicinmybrain
Copy link
Contributor

We see xpasses in Fedora with Python 3.11.1:

Just to clarify, this isn’t specific to Python 3.11, as it was working in Fedora 37 at one point, and I see the same thing on Fedora 36 with Python 3.10.9. Perhaps it was triggered by a dependency update.

@Dreamsorcerer
Copy link
Member

@webknjaz usually has some opinions on xfail. Personally, these tests look pretty useless to me, they are testing something that we shouldn't be supporting (atleast by Python 3.7). So, I'm happy to merge this unless @webknjaz says otherwise (I'd also be happy just removing the tests).

I am curious why it would succeed on Fedora though. The only way I can imagine it passing is if Fedora is patching Python, or otherwise modifying its behaviour, so it doesn't emit those deprecation warnings.

@hroncok
Copy link
Author

hroncok commented Jan 27, 2023

We have backported this: python/cpython#100412 -- but the tests started to xpass even sooner :/

@webknjaz
Copy link
Member

Looks like xfails are conditional on python 3.10+. If the CI starts to pass on the latest patch versions of CPython for everything in the matrix, I don't see why we'd need to keep the xfail marks at all.

@hroncok FYI we've pretty much decided not to make another release from the 3.8 branch, so this PR should probably be targeting 3.9 and master instead. Of course, we can merge the PR into 3.8 if needed for downstream, but this doesn't mean that it'll ever get released from this branch.

@webknjaz
Copy link
Member

We have backported this: python/cpython#100412 -- but the tests started to xpass even sooner :/

Based on the comment in that PR, this will start xfailing again in Python 3.12. So maybe we just need to change the xfail conditional to py3.12+

@hroncok
Copy link
Author

hroncok commented Jan 28, 2023

@hroncok FYI we've pretty much decided not to make another release from the 3.8 branch, so this PR should probably be targeting 3.9 and master instead. Of course, we can merge the PR into 3.8 if needed for downstream, but this doesn't mean that it'll ever get released from this branch.

We've already patched the Fedora package to do this, just trying to be good citizens here and offer the patch upstream instead of keeping it downstream-only. Take it, leave it, put it somewhere else, delete the tests, or do whatever you see fit :)

We would love to be eventually able to drop the patch from Fedora, but if that's in a 3.9.x release, that's fine.

archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Apr 13, 2023
aio-libs/aiohttp#7178 (comment)

git-svn-id: file:///srv/repos/svn-community/svn@1445521 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Apr 13, 2023
aio-libs/aiohttp#7178 (comment)


git-svn-id: file:///srv/repos/svn-community/svn@1445521 9fca08f4-af9d-4005-b8df-a31f2cc04f65
@webknjaz webknjaz marked this pull request as ready for review July 7, 2023 22:13
@webknjaz webknjaz requested a review from asvetlov as a code owner July 7, 2023 22:13
@webknjaz webknjaz added the bot:chronographer:skip This PR does not need to include a change note label Jul 7, 2023
We see xpasses in Fedora with Python 3.11.1 or 3.10.9:

    =================================== FAILURES ===================================
    __________________________ test_default_loop[pyloop] ___________________________
    [XPASS(strict)] No idea why ClientRequest() is constructed out of loop but it calls `asyncio.get_event_loop()`
    ____________________ TestStreamReader.test_ctor_global_loop ____________________
    [XPASS(strict)] No idea why ClientRequest() is constructed out of loop but it calls `asyncio.get_event_loop()`
    __________________________ test_set_loop_default_loop __________________________
    [XPASS(strict)] No idea why _set_loop() is constructed out of loop but it calls `asyncio.get_event_loop()`
@codecov
Copy link

codecov bot commented Jul 7, 2023

Codecov Report

Merging #7178 (a360089) into 3.8 (81ba8aa) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              3.8    #7178      +/-   ##
==========================================
- Coverage   97.40%   97.38%   -0.02%     
==========================================
  Files         107      107              
  Lines       30996    30996              
  Branches     3924     3924              
==========================================
- Hits        30191    30186       -5     
- Misses        602      605       +3     
- Partials      203      205       +2     
Flag Coverage Δ
CI-GHA 97.25% <ø> (-0.02%) ⬇️
OS-Linux 96.90% <ø> (-0.01%) ⬇️
OS-Windows 95.23% <ø> (-0.01%) ⬇️
OS-macOS 96.64% <ø> (-0.02%) ⬇️
Py-3.10.11 94.84% <ø> (-2.02%) ⬇️
Py-3.10.12 96.49% <ø> (?)
Py-3.11.0 96.21% <ø> (ø)
Py-3.6.15 96.37% <ø> (-0.01%) ⬇️
Py-3.6.8 94.76% <ø> (-0.01%) ⬇️
Py-3.7.16 ?
Py-3.7.17 96.53% <ø> (?)
Py-3.7.9 ?
Py-3.8.10 94.82% <ø> (?)
Py-3.8.16 ?
Py-3.8.17 96.49% <ø> (?)
Py-3.9.13 94.81% <ø> (ø)
Py-3.9.16 ?
Py-3.9.17 96.46% <ø> (?)
Py-pypy7.3.11 93.94% <ø> (-0.01%) ⬇️
VM-macos-latest 96.64% <ø> (-0.02%) ⬇️
VM-ubuntu-20.04 96.89% <ø> (ø)
VM-ubuntu-latest 96.43% <ø> (-0.01%) ⬇️
VM-windows-latest 95.23% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
tests/test_client_request.py 99.60% <ø> (ø)
tests/test_streams.py 100.00% <ø> (ø)
tests/test_web_app.py 98.57% <ø> (ø)

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@webknjaz webknjaz enabled auto-merge (squash) July 7, 2023 22:24
@webknjaz webknjaz merged commit b1fbb49 into aio-libs:3.8 Jul 7, 2023
29 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:skip This PR does not need to include a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants