Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

An easier way to have test cases in built-in functions #11388

Open
joankaradimov opened this issue Sep 4, 2023 · 6 comments
Open

An easier way to have test cases in built-in functions #11388

joankaradimov opened this issue Sep 4, 2023 · 6 comments
Labels
topic: collection related to the collection phase type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature

Comments

@joankaradimov
Copy link

joankaradimov commented Sep 4, 2023

I'd like to suggest that calls to inspect.isfunction are replaced with inspect.isroutine.

What's the problem this feature will solve?

I'm trying to test some native code functions I have written in Cython. Those are either coming from .pxd files or are cdef functions in .pyx files. These are not visible to Python code. They can only be called from Cython functions (which are built-in functions).

Describe the solution you'd like

I'd like to add a conftest.py file which defines in its pytest_collect_file how to load a Cython test module and then write test_* functions in Cython which call the said native functions.

Alternative Solutions

It is currently impossible to have a test_* function be a built-in function. It fails the check for inspect.isfunction defined in src/_pytest/python.py. As a workaround I copy-paste the entire pytest_pycollect_makeitem and replace its isfunction checks with isroutine checks. This is enough of a workaround to get my test cases loaded.

Additional context

It's unfortunate that I have to copy-paste library code. This goes against the whole idea of having hooks. One solution is to add new hooks. Another is to just allow a wider range of callables to be detected as test functions.

@RonnyPfannschmidt
Copy link
Member

as pytest is doing certain tasks in unpacking the method to call, is not immediately clear if this is safe to do in general

unfortunately definition collection is not yet part of the collection tree else the code could be simplified to making a own Definition and have pytest handle the rest

@joankaradimov
Copy link
Author

Sounds like I'm stuck with the pytest_pycollect_makeitem hook.

Is there something I can do to investigate if this is safe to use isroutine with method unpacking?

@RonnyPfannschmidt
Copy link
Member

@joankaradimov for now we might be able to get away with a opt in for allowing built-ins as tests
the isbuiltin check is a little less broad, and if its a opt-in its not a general concern for now

@joankaradimov
Copy link
Author

Thanks! I can make a PR for that.

Is an additional command line argument sufficient? I can document it in the collection section of the help. As for the naming -- I'm thinking something along the lines of --keep-builtins or --collect-builtins.

Any other input?

@RonnyPfannschmidt
Copy link
Member

I believe a option for the config file fit's better as its always needed when it's needed

@Zac-HD Zac-HD added type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature topic: collection related to the collection phase labels Sep 7, 2023
@ev-br
Copy link

ev-br commented Sep 26, 2023

As suggested in #11450 (reply in thread) (which was marked as a duplicate of this one), I'd add that for at least one use-case (collecting doctests), just using inspect.isroutine would be not enough: not all C-implemented objects respond to isroutine.

At least numpy ufuncs do not --- copying from scipy/scipy_doctest#99 (comment):

In [42]: from scipy import special as s

In [43]: import numpy as np

In [57]: for obj in [s.erf, np.add, LinearNDInterpolator, LinearNDInterpolator.__call__]:
    ...:     print(obj, inspect.isfunction(obj), inspect.isroutine(obj), inspect.isbuilt
    ...: in(obj), inspect.isclass(obj), isinstance(obj, np.ufunc))
    ...:
<ufunc 'erf'> False False False False True
<ufunc 'add'> False False False False True
<class 'scipy.interpolate.interpnd.LinearNDInterpolator'> False False False True False
<cyfunction NDInterpolatorBase.__call__ at 0x7fa04ee17440> False True False False False

So it would be great if there's a hook or a way to pass a predicate.

A (possibly tangentially) related issue is that collecting doctests from compiled modules fails with

ERROR: found no collectors for /home/br/repos/scipy/scipy/build-install/lib/python3.10/site-packages/scipy/interpolate/interpnd.cpython-310-x86_64-linux-gnu.so

(#11450 has a proper repro)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: collection related to the collection phase type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature
Projects
None yet
Development

No branches or pull requests

4 participants