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

Add Python bindings coverage testing. #253

Closed
hsorby opened this issue Jul 23, 2018 · 6 comments
Closed

Add Python bindings coverage testing. #253

hsorby opened this issue Jul 23, 2018 · 6 comments

Comments

@hsorby
Copy link
Contributor

hsorby commented Jul 23, 2018

With the (recent) addition of the Python bindings I think adding coverage testing on the bindings would be beneficial.

@MichaelClerx
Copy link
Contributor

MichaelClerx commented Jul 23, 2018

Agreed, but not sure how. Most of the code is generated and will account for lots of edge cases, I'm guessing. Really the only thing we want to check is:

  1. Is every C++ method exposed (keeping in mind that overloaded C++ methods map to a single Python expression).
  2. Did we add a docstring. Done: see below

Someone'll have to do some googling to see what the SWIG people have come up with!

@hsorby
Copy link
Contributor Author

hsorby commented Jul 23, 2018

I was thinking that we would want to check that all methods that are exposed are used at least once.

Not sure how we would go about testing for the presence of a docstring?

from libcellml import Reset
len(Reset.addWhen.__doc__) > 0

maybe iterating all available methods and checking __doc__ is at least something?

@nickerso
Copy link
Contributor

I agree that having coverage testing to make sure the full libCellML API is exposed and tested via the bindings. Perhaps checking docstrings can be part of a more comprehensive style checking/linting that we should also have one day - if such checking included API documentation then it would be safe to assume the docs make it through to the bindings?

@hsorby
Copy link
Contributor Author

hsorby commented Jul 24, 2018

The full libCellML API is not exposed we do have to suppress some of the C++ API because the interpretation of the overloaded functions doesn't translate well.

The documentation for bindings is manually put in place.

@MichaelClerx
Copy link
Contributor

I've added a test for missing docstrings, as part of the work checking/updating the imports: 2c306e1

@MichaelClerx
Copy link
Contributor

Turns out it also works as an unofficial cover test: When the C++ API changes the Python API gets updated automatically, and this test will complain that the new Python class/function is undocumented. If this happens, it's very likely the class/function won't have a test either!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants