bpo-30524: Write unit tests for FASTCALL#2022
bpo-30524: Write unit tests for FASTCALL#2022vstinner merged 1 commit intopython:masterfrom vstinner:test_fastcall
Conversation
Modules/_testcapimodule.c
Outdated
There was a problem hiding this comment.
I would remove the test_ prefix (and may be even pyobject_). Functions with the test_ prefix usually do all testing internally, they don't return a value for checking in Python test.
There was a problem hiding this comment.
"test_" is a prefix to avoid name conflict with the called function: "pyobject_fastcall" vs "_PyObject_FastCall". It avoids confusion when you have to debug _testcapi.
Modules/_testcapimodule.c
Outdated
There was a problem hiding this comment.
Unfortunately this can't be used for testing the case nargs == 0, stack != NULL. Maybe set stack to NULL only when pass None as args?
There was a problem hiding this comment.
As the author of _PyObject_FastCall(), I prefer to test passing NULL since I expect more bugs for stack == NULL, like the _PyStack_UnpackDict() bug.
I don't think that it's worth it to test calling _PyObject_FastCall() with nargs==0 and stack != NULL.
There was a problem hiding this comment.
Bugs hide in untested corners.
Lib/test/test_call.py
Outdated
There was a problem hiding this comment.
I think expected=expected is redundant. func and args are enough for identifying the test case, and expected is reported if check_result() fails.
Lib/test/test_call.py
Outdated
There was a problem hiding this comment.
Maybe test also _testcapi.pyobject_fastcalldict(func, args, {})?
Lib/test/test_call.py
Outdated
There was a problem hiding this comment.
This case (and the case for pyfunc_noarg) can be removed if add testing _testcapi.pyobject_fastcalldict(func, args, {}) and _testcapi.pyobject_fastcallkeywords(func, args, {}) for CALLS_POSARGS.
Modules/_testcapimodule.c
Outdated
There was a problem hiding this comment.
Bugs hide in untested corners.
Test C functions: * _PyObject_FastCall() * _PyObject_FastCallDict() * _PyObject_FastCallKeywords()
|
I rewrote my change to test much more cases. Most (or all?) possible ways to call FastCall functions should now be tested. |
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Would be nice to backport tests at least to 3.6. This could expose 3.6 bugs fixed in 3.7.
Sure, I wrote this change for 3.6, to test the datetime.datetime.now() bug :-) But obvious, firstI wrote it for master. |
Resolve conflicts: 3b5cf85 bpo-30524: Write unit tests for FASTCALL (python#2022)
3 participants
Test C functions: