-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
TST: Add CMake and gtest frameworks #4
base: main
Are you sure you want to change the base?
Conversation
cfe4ed3
to
a93a0b0
Compare
Thanks @inkydragon. This aligns well with what I had planned. My thoughts for testing is that we should have scripts that generate csv files of test cases. The C++ tests would then read these files, and we would have different tiers of tests. Some running on every push to a PR, and others (like GPU jobs or more extensive tests) running on a cron. For the short term I plan to stash the csv files in GitHub, but eventually they would likely go somewhere better suited for file storage. I'll start generating some files this week, and if you want to continue this, you can either write something to pull them in for the tests, or I can follow through on what you're setting up to do that. |
tests/CMakeLists.txt
Outdated
FetchContent_Declare( | ||
googletest | ||
URL https://github.com/google/googletest/releases/download/v1.15.2/googletest-1.15.2.tar.gz | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Locally I see
CMake Warning (dev) at .pixi/envs/dev/share/cmake-3.30/Modules/FetchContent.cmake:1373 (message):
The DOWNLOAD_EXTRACT_TIMESTAMP option was not given and policy CMP0135 is
not set. The policy's OLD behavior will be used. When using a URL
download, the timestamps of extracted files should preferably be that of
the time of extraction, otherwise code that depends on the extracted
contents might not be rebuilt if the URL changes. The OLD behavior
preserves the timestamps from the archive instead, but this is usually not
what you want. Update your project to the NEW behavior or specify the
DOWNLOAD_EXTRACT_TIMESTAMP option with a value of true to avoid this
robustness issue.
Call Stack (most recent call first):
tests/CMakeLists.txt:4 (FetchContent_Declare)
This warning is for project developers. Use -Wno-dev to suppress it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switch to use the commit id to eliminate this warning
Feel free to open a new pr based on this. I guess there are a lot of pr in scipy waiting for your review. My previous plan was to manually write some test cases to achieve higher line of code coverage with a small number of tests. |
I'm sure y'all thought about this already, but I'll ask nonetheless: have you considered adding a thin python layer for testing and using python testing frameworks? While I don't have a horse in this race, the setup otherwise seems suspiciously of the "generate text data and use gnuplot" type of construction which got ditched once somebody showed me |
Yeah, using Python is the other option I've considered. I suggested that in #1 because I thought it would be the easiest way to get started. In any case, I'd want us to generate test cases and store them somewhere because generating arbitrary precision reference data takes several orders of magnitude longer than running the C++ code. In terms of how to serialize the test cases, I like text formats, because then it's clear they are what they say they are. Binary files could be compromised potentially. If the test cases are coming from files, and the C++ tests just read files, call the function on each test case, and compare to the reference value, I think that's pretty straightforward. Having a C++ testing framework also makes it easier to test internal components which aren't so easy to wrap in Python like ufuncs are. I'm really happy to see @inkydragon's interest in helping us get set up with CMake and gtest. |
This is a POC that currently sets up a basic CMake + gtest test environment.
Build and generating coverage report:
Sample:
73c842f
(#1)because the test code only calls the airy-related functions