GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||
GeezerVison is showing github.com — gh-106368: Argument Clinic: Add tests for cloned functions with custom C base names by erlend-aasland · Pull Request #107977 · python/cpython Conversation
|
| if (is_legal_py_identifier(full_name) and | |
| (not c_basename or is_legal_c_identifier(c_basename)) and | |
| is_legal_py_identifier(existing)): |
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?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Ah, and it looks like the CI is complaining about the same thing: https://github.com/python/cpython/actions/runs/5868181574/job/15910445072 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I wonder why we're not seeing complaints about |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
... 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). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Ah, good call. On it! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Likewise! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.