Skip to content

Shorted fixture parametrization ids#1192

Merged
asvetlov merged 4 commits intoaio-libs:masterfrom
asvetlov:short-ids
Jun 23, 2025
Merged

Shorted fixture parametrization ids#1192
asvetlov merged 4 commits intoaio-libs:masterfrom
asvetlov:short-ids

Conversation

@asvetlov
Copy link
Member

@asvetlov asvetlov commented Jun 23, 2025

The change significantly reduces generated test names to fit better on the local terminal, in logs, and benchmark web view.

For example, test_keys_view_xor[case-insensitive-pure-python-module] becomes test_keys_view_xor[ci-py], test_keys_view_xor[case-sensitive-c-extension-module] becomes test_keys_view_xor[cs-c]

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Jun 23, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Jun 23, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #1192 will not alter performance

Comparing asvetlov:short-ids (090d511) with master (7e68e1b)

Summary

✅ 4 untouched benchmarks
🆕 240 new benchmarks
⁉️ 240 (👁 240) dropped benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
👁 test_cimultidict_add_istr[c-extension-module] 3.5 ms N/A N/A
🆕 test_cimultidict_add_istr[c] N/A 3.6 ms N/A
👁 test_cimultidict_add_istr[pure-python-module] 97 ms N/A N/A
🆕 test_cimultidict_add_istr[py] N/A 101.5 ms N/A
👁 test_cimultidict_delitem_istr[c-extension-module] 64 µs N/A N/A
🆕 test_cimultidict_delitem_istr[c] N/A 64.7 µs N/A
👁 test_cimultidict_delitem_istr[pure-python-module] 945.4 µs N/A N/A
🆕 test_cimultidict_delitem_istr[py] N/A 979.2 µs N/A
👁 test_cimultidict_extend_istr[c-extension-module] 2.9 ms N/A N/A
🆕 test_cimultidict_extend_istr[c] N/A 2.9 ms N/A
👁 test_cimultidict_extend_istr[pure-python-module] 159.1 ms N/A N/A
🆕 test_cimultidict_extend_istr[py] N/A 161.7 ms N/A
👁 test_cimultidict_extend_istr_with_kwargs[c-extension-module] 7.5 ms N/A N/A
🆕 test_cimultidict_extend_istr_with_kwargs[c] N/A 7.5 ms N/A
👁 test_cimultidict_extend_istr_with_kwargs[pure-python-module] 268.3 ms N/A N/A
🆕 test_cimultidict_extend_istr_with_kwargs[py] N/A 269.4 ms N/A
👁 test_cimultidict_fetch_istr[c-extension-module] 48 µs N/A N/A
🆕 test_cimultidict_fetch_istr[c] N/A 48.4 µs N/A
👁 test_cimultidict_fetch_istr[pure-python-module] 489.7 µs N/A N/A
🆕 test_cimultidict_fetch_istr[py] N/A 510.8 µs N/A
... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

@asvetlov asvetlov marked this pull request as ready for review June 23, 2025 07:10
@codecov
Copy link

codecov bot commented Jun 23, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.31%. Comparing base (7e68e1b) to head (090d511).
⚠️ Report is 52 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1192   +/-   ##
=======================================
  Coverage   98.31%   98.31%           
=======================================
  Files          27       27           
  Lines        3856     3856           
  Branches      704      704           
=======================================
  Hits         3791     3791           
  Misses         18       18           
  Partials       47       47
Flag Coverage Δ
CI-GHA 98.31% <100.00%> (ø)
MyPy 80.38% <100.00%> (ø)
pytest 99.85% <100.00%> (ø)

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.

@asvetlov asvetlov merged commit 5ab49a5 into aio-libs:master Jun 23, 2025
64 checks passed
@asvetlov asvetlov deleted the short-ids branch June 23, 2025 07:24
@webknjaz
Copy link
Member

webknjaz commented Jun 23, 2025

FTR, I still think that choosing poor names for things reduces readability, as I said in #1157 earlier.

@asvetlov
Copy link
Member Author

asvetlov commented Jun 23, 2025

@webknjaz the PR doesn't change fixture names, but their parameterization IDs only.

Please, let's try to live with these names. They fit UI screens much better than before.

@webknjaz
Copy link
Member

webknjaz commented Jun 23, 2025

I know. But this was the most annoying bit of that PR for me. I see that it's shorter now. I just don't agree that it's better. It just changed one minor annoyance into something worse in a different place 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@asvetlov @webknjaz