Skip to content

ENH: Adding cmocka for unit testing #21304

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

Open
HaoZeke opened this issue Apr 6, 2022 · 2 comments
Open

ENH: Adding cmocka for unit testing #21304

HaoZeke opened this issue Apr 6, 2022 · 2 comments
Assignees
Labels
05 - Testing component: numpy.f2py triaged Issue/PR that was discussed in a triage meeting

Comments

@HaoZeke
Copy link
Member

HaoZeke commented Apr 6, 2022

Proposed new feature or change: Unit testing C code

One of the primary motivations behind this change is to support F2PY generated codes better. Working with fortranobject.{c,h} while trying to have test coverage is effectively impossible right now. Currently we only have integration tests (at the Python level) and this makes it hard to track problems sometimes.

However, beyond F2PY as well, unit testing the NumPy-C API can be done in low-boilerplate way with cmocka.

Some nice features of cmocka:

  • C only
  • Surprisingly low boilerplate
  • Is easily available on conda
  • Used by a bunch of other projects (e.g. libssh)

Existing C-testing:

  • Integration (pytest)
  • Making small extension modules to test parts of the public API (test_mem_policy.py)

Some questions which have come up in discussions are:

  • Compiling on Windows is slow --> we can concatenate these on the CI
  • Compiling them in monolithic files might be hard to debug (e.g. F2PY generates wrong C code)
    • This should not be an issue for the most part, compiler errors are caught by our test suite now as well, which should not change.
@HaoZeke HaoZeke added 05 - Testing component: numpy.f2py 54 - Needs decision triage review Issue/PR to be discussed at the next triage meeting triaged Issue/PR that was discussed in a triage meeting and removed 54 - Needs decision triage review Issue/PR to be discussed at the next triage meeting labels Apr 6, 2022
@HaoZeke
Copy link
Member Author

HaoZeke commented Apr 6, 2022

This was discussed in this triage meeting. Cliff notes:

  • Should integrate with runtests.py (this means some nice statistics and also a flag)
  • Conditionally alright, especially if optional
  • Watch build times and also make sure the right flags are passed in the build system
  • Stick to only F2PY for now, perhaps more later
  • What about C++?
    • We use extern C in any case, so a large subset can be tested
    • Also C++ testing libraries release rapidly and depend on newer compiler versions (e.g. Catch2, GTests, etc)

Some precedent:

  • doxygen was added as an optional docs-only dependency

@HaoZeke
Copy link
Member Author

HaoZeke commented Nov 25, 2023

Most of the C code isn't tested on the CI either (#21304), which is how some other strange bugs (#25186 (comment)) sneak in. Also part of #20053.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
05 - Testing component: numpy.f2py triaged Issue/PR that was discussed in a triage meeting
Projects
None yet
Development

No branches or pull requests

1 participant