gh-106368: Argument Clinic: Add tests for cloned functions with custom C base names #107977
Conversation
…ustom C base name
There was a problem hiding this comment.
There is still one case in this block that is untested:
cpython/Tools/clinic/clinic.py
Lines 4888 to 4890 in e28b0dc
What if is_legal_py_identifier(full_name) evaluates to True, not c_basename evaluates to False, is_legal_c_identifier(c_basename) evaluates to False and is_legal_py_identifier(existing) evaluates to True?
I.e., if I apply this diff to your PR branch, the assertion is never triggered:
- if (is_legal_py_identifier(full_name) and
- (not c_basename or is_legal_c_identifier(c_basename)) and
- is_legal_py_identifier(existing)):
+ if is_legal_py_identifier(full_name) and is_legal_py_identifier(existing):
+ if c_basename:
+ assert is_legal_c_identifier(c_basename)...Could you also add a test with a cloned function with a custom C base name where the custom C base name is not a legal C identifier?
|
I'm getting warnings about the execution environment when I'm running Ran 243 tests in 0.620s
OK
Warning -- files was modified by test_clinic
Warning -- Before: []
Warning -- After: ['clinic/']
Warning -- files was modified by test_clinic
Warning -- Before: []
Warning -- After: ['clinic/']
test_clinic failed (env changed)
== Tests result: SUCCESS ==
1 test altered the execution environment:
test_clinic
Total duration: 1.2 sec
Tests result: SUCCESS |
Ah, and it looks like the CI is complaining about the same thing: https://github.com/python/cpython/actions/runs/5868181574/job/15910445072 |
Ah, yeah, I noticed earlier today, but got sidetracked. Since we're running a "expect success" test, we will actually generate clinic output in the added test (hence the sudden |
|
I wonder why we're not seeing complaints about |
... because they don't create output; either they're dumping to block or buffer, or they're testing some weird directive, or they're run with precomputed checksums in the input (hence no regenerated output). |
Ah, good call. On it! |
Likewise! |
Uh oh!
There was an error while loading. Please reload this page.