Skip to content

Conversation

kumaraditya303
Copy link
Contributor

What do these changes do?

Are there changes in behavior for the user?

Related issue number

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes

Copy link

codspeed-hq bot commented Aug 21, 2025

CodSpeed Performance Report

Merging #1235 will not alter performance

Comparing kumaraditya303:master (e751492) with master (e0e61c2)

Summary

✅ 245 untouched benchmarks

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Aug 21, 2025
Copy link

codecov bot commented Aug 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.85%. Comparing base (e0e61c2) to head (e751492).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1235      +/-   ##
==========================================
- Coverage   99.85%   99.85%   -0.01%     
==========================================
  Files          26       26              
  Lines        3510     3507       -3     
  Branches      252      252              
==========================================
- Hits         3505     3502       -3     
  Misses          3        3              
  Partials        2        2              
Flag Coverage Δ
CI-GHA 99.85% <100.00%> (-0.01%) ⬇️
pytest 99.85% <100.00%> (-0.01%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@Vizonex Vizonex left a comment

Choose a reason for hiding this comment

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

I thought something was altered from the tests I've added but this doesn't seem to be the case, glad this has now been added in this should be good for stress-testing the 3.14 update.

@Vizonex
Copy link
Contributor

Vizonex commented Aug 21, 2025

@kumaraditya303 I did not notice this earlier but it seems some tests are failing for some reason let me go in and see if I can suggest any changes. The others are on a tighter schedule than me apparently.

@kumaraditya303
Copy link
Contributor Author

I did not notice this earlier but it seems some tests are failing for some reason let me go in and see if I can suggest any changes. The others are on a tighter schedule than me apparently.

The failing tests are leak tests, it is likely that it is because of gc changes in 3.14 or just higher memory usage on 3.14 interpreter though I haven't looked at it closely.

@Vizonex
Copy link
Contributor

Vizonex commented Aug 24, 2025

I did not notice this earlier but it seems some tests are failing for some reason let me go in and see if I can suggest any changes. The others are on a tighter schedule than me apparently.

The failing tests are leak tests, it is likely that it is because of gc changes in 3.14 or just higher memory usage on 3.14 interpreter though I haven't looked at it closely.

Makes sense. I mainly implemented the newer memory-leak test to prevent newer memory leak issues in the future since there have been several incidents in MultiDict's history of this happening repetitively.

I am surprised it's not related to that one and only istr. When tests/test_istr.py::test_leak[py] is ran in isolation it passes which I find very strange.

@Vizonex
Copy link
Contributor

Vizonex commented Aug 30, 2025

@kumaraditya303 Great job, I'll be sure to contact one of the contributors over on matrix If I don't see any activity here in the next couple days.

@kumaraditya303
Copy link
Contributor Author

Great job, I'll be sure to contact one of the contributors over on matrix If I don't see any activity here in the next couple days.

Thanks I have updated the tests for CPython 3.14, specifically these two changes:

  • tests/test_istr.py: Changed it to check for case_insensitive_str_class instances rather than all objects which isn't stable.
  • tests/isolated/multidict_pop.py: Change it to check for relative increase in memory rather than absolute memery usage.

@kumaraditya303 kumaraditya303 marked this pull request as ready for review August 30, 2025 05:09
Copy link
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

@bdraco bdraco closed this Aug 30, 2025
@bdraco bdraco reopened this Aug 30, 2025
@bdraco bdraco merged commit 18344e2 into aio-libs:master Aug 30, 2025
143 of 145 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants