gh-95065: Argument Clinic: Add functional tests of deprecated positionals by erlend-aasland · Pull Request #107768 · python/cpython · GitHub

Move the "deprecated positinal" tests from clinic.test.c to
_testclinic.c. Mock PY_VERSION_HEX in order to prevent the generated
compiler warnings/errors to trigger. Put clinic code for deprecated
positionals in Modules/clinic/_testclinic_depr_star.c.h for easy
inspection of the generated code.

Issue: Argument Clinic: add support for deprecating positional use of parameters #95065

…ositionals Move the "deprecated positinal" tests from clinic.test.c to _testclinic.c. Mock PY_VERSION_HEX in order to prevent the generated compiler warnings/errors to trigger. Put clinic code for deprecated positionals in Modules/clinic/_testclinic_depr_star.c.h for easy inspection of the generated code.

The big diff is caused by moving the generated clinic code.

@serhiy-storchaka, IMO this is better than visual inspection of the generated code. We want to prevent regressions because of future refactorings, features and bugfixes.

The CI currently isn't running on this PR due to merge conflicts -- I tried building with it locally, but it causes build failures on Windows:

Build FAILED.

C:\Users\alexw\coding\cpython\Modules\clinic\_testclinic_depr_star.c.h(11,1): error C2133: 'deprecate_positional_pos0_len1__doc__': unknown size [C:\Users\
alexw\coding\cpython\PCbuild\_testclinic.vcxproj]
C:\Users\alexw\coding\cpython\Modules\clinic\_testclinic_depr_star.c.h(98,1): error C2133: 'deprecate_positional_pos0_len2__doc__': unknown size [C:\Users\
alexw\coding\cpython\PCbuild\_testclinic.vcxproj]
C:\Users\alexw\coding\cpython\Modules\clinic\_testclinic_depr_star.c.h(188,1): error C2133: 'deprecate_positional_pos0_len3_with_kwd__doc__': unknown size
[C:\Users\alexw\coding\cpython\PCbuild\_testclinic.vcxproj]
C:\Users\alexw\coding\cpython\Modules\clinic\_testclinic_depr_star.c.h(287,1): error C2133: 'deprecate_positional_pos1_len1_optional__doc__': unknown size
[C:\Users\alexw\coding\cpython\PCbuild\_testclinic.vcxproj]
C:\Users\alexw\coding\cpython\Modules\clinic\_testclinic_depr_star.c.h(383,1): error C2133: 'deprecate_positional_pos1_len1__doc__': unknown size [C:\Users
\alexw\coding\cpython\PCbuild\_testclinic.vcxproj]
C:\Users\alexw\coding\cpython\Modules\clinic\_testclinic_depr_star.c.h(473,1): error C2133: 'deprecate_positional_pos1_len2_with_kwd__doc__': unknown size
[C:\Users\alexw\coding\cpython\PCbuild\_testclinic.vcxproj]
C:\Users\alexw\coding\cpython\Modules\clinic\_testclinic_depr_star.c.h(572,1): error C2133: 'deprecate_positional_pos2_len1__doc__': unknown size [C:\Users
\alexw\coding\cpython\PCbuild\_testclinic.vcxproj]
C:\Users\alexw\coding\cpython\Modules\clinic\_testclinic_depr_star.c.h(664,1): error C2133: 'deprecate_positional_pos2_len2__doc__': unknown size [C:\Users
\alexw\coding\cpython\PCbuild\_testclinic.vcxproj]
C:\Users\alexw\coding\cpython\Modules\clinic\_testclinic_depr_star.c.h(758,1): error C2133: 'deprecate_positional_pos2_len3_with_kwd__doc__': unknown size
[C:\Users\alexw\coding\cpython\PCbuild\_testclinic.vcxproj]
    0 Warning(s)
    9 Error(s)

The CI currently isn't running on this PR due to merge conflicts -- I tried building with it locally, but it causes build failures on Windows:

Does it still happen?

The output directive has a bug that prevents us from specifying the stack index. We should fix that in a separate PR.

The CI currently isn't running on this PR due to merge conflicts -- I tried building with it locally, but it causes build failures on Windows:

Does it still happen?

Nope! Builds fine locally now :)

Would not be better to use version like 100.200 to avoid rewriting tests every year?

Would not be better to use version like 100.200 to avoid rewriting tests every year?

No need, we mock the version.

Do the test fail if change the stacklevel argument in PyErr_WarnEx() from 1 to 2?

Do the test fail if change the stacklevel argument in PyErr_WarnEx() from 1 to 2?

They do not. The same filename is produced for both stack levels.

And wrong stacklevel is a common error. Even your original code did have it wrong initially. Therefore, I think it is important to check it using one of the methods I suggested above.

Note: we will get merge conflicts from #107808.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

I agree with Serhiy that explicitly testing the stacklevel would be good. Other than that, this is looking much better now, thanks! The readabiliy is much improved :)

I'm happy if Serhiy's happy!

Few nitpicks and LGTM.

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Thank you Serhiy and Alex; this PR is in much better shape now! With this, we've got a nice framework for similar tests in place. I'll start working on what's left in clinic.test.c.

erlend-aasland requested a review from AlexWaygood as a code owner August 8, 2023 09:28

erlend-aasland added the skip news label Aug 8, 2023

erlend-aasland requested review from serhiy-storchaka and sobolevn August 8, 2023 09:28

bedevere-bot mentioned this pull request Aug 8, 2023

Argument Clinic: add support for deprecating positional use of parameters #95065

Closed

bedevere-bot added the awaiting core review label Aug 8, 2023

erlend-aasland commented Aug 8, 2023

View reviewed changes

Comment thread Lib/test/test_clinic.py Outdated

AlexWaygood reviewed Aug 8, 2023

View reviewed changes

Comment thread Lib/test/test_clinic.py Outdated

erlend-aasland added 3 commits August 8, 2023 21:28

Fix tests

a28c62e

Readable tests

61bef45

Pull in main

daea15e

erlend-aasland requested a review from AlexWaygood August 8, 2023 20:04

erlend-aasland commented Aug 9, 2023

View reviewed changes

AlexWaygood reviewed Aug 9, 2023

View reviewed changes

Comment thread Lib/test/test_clinic.py Outdated

serhiy-storchaka reviewed Aug 9, 2023

View reviewed changes

Comment thread Lib/test/test_clinic.py Outdated

Comment thread Lib/test/test_clinic.py Outdated

Comment thread Lib/test/test_clinic.py Outdated

erlend-aasland added 4 commits August 9, 2023 13:15

Pull in main

00a7562

Address review: greatly simplify tests

6559443

More accurate regex; pass param names explicitly

194420b

Even nicer

6875b2f

serhiy-storchaka reviewed Aug 9, 2023

View reviewed changes

Merge branch 'main' into clinic/better-depr-tests

181cbf7

AlexWaygood reviewed Aug 9, 2023

View reviewed changes

Comment thread Lib/test/test_clinic.py Outdated

Update Lib/test/test_clinic.py

9f40557

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

AlexWaygood reviewed Aug 9, 2023

View reviewed changes

erlend-aasland added 4 commits August 9, 2023 23:36

Also test __new__ and __init__

2e3cdfc

Check the line number

cdf0574

Pull in main

91c122a

Update globals-to-fix.tsv

9a4128d

AlexWaygood approved these changes Aug 9, 2023

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Aug 9, 2023

serhiy-storchaka approved these changes Aug 10, 2023

View reviewed changes

Comment thread Lib/test/test_clinic.py Outdated

Comment thread Lib/test/test_clinic.py Outdated

Comment thread Lib/test/test_clinic.py Outdated

Comment thread Modules/_testclinic.c Outdated

Comment thread Lib/test/test_clinic.py

erlend-aasland and others added 2 commits August 10, 2023 08:52

Apply suggestions from code review

e56888c

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Merge branch 'main' into clinic/better-depr-tests

44aac33

erlend-aasland enabled auto-merge (squash) August 10, 2023 06:54

erlend-aasland merged commit 39ef93e into python:main Aug 10, 2023

bedevere-bot removed the awaiting merge label Aug 10, 2023

erlend-aasland deleted the clinic/better-depr-tests branch August 10, 2023 07:35

+              output push
+              destination deprstar new file '{dirname}/clinic/_testclinic_depr_star.c.h'
+              output everything deprstar
+              #output methoddef_ifndef buffer 1

gh-95065: Argument Clinic: Add functional tests of deprecated positionals by erlend-aasland · Pull Request #107768 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-95065: Argument Clinic: Add functional tests of deprecated positionals - #107768

gh-95065: Argument Clinic: Add functional tests of deprecated positionals#107768erlend-aasland merged 16 commits intopython:mainpython/cpython:mainfrom erlend-aasland:clinic/better-depr-testserlend-aasland/cpython:clinic/better-depr-testsCopy head branch name to clipboard

Conversation

erlend-aasland commented Aug 8, 2023 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erlend-aasland commented Aug 8, 2023

Uh oh!

Uh oh!

erlend-aasland commented Aug 8, 2023

Uh oh!

AlexWaygood commented Aug 8, 2023

Uh oh!

Uh oh!

erlend-aasland commented Aug 8, 2023

Uh oh!

erlend-aasland Aug 9, 2023

Choose a reason for hiding this comment

Uh oh!

AlexWaygood commented Aug 9, 2023

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erlend-aasland commented Aug 9, 2023

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

erlend-aasland commented Aug 9, 2023

Uh oh!

serhiy-storchaka commented Aug 9, 2023

Uh oh!

erlend-aasland commented Aug 9, 2023

Uh oh!

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erlend-aasland commented Aug 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gh-95065: Argument Clinic: Add functional tests of deprecated positionals#107768
erlend-aasland merged 16 commits into
python:mainfrom
erlend-aasland:clinic/better-depr-tests

erlend-aasland commented Aug 8, 2023 •
edited by bedevere-bot
Loading