kevingurney opened a new pull request, #37784:
URL: https://github.com/apache/arrow/pull/37784

   This pull request removes `GoogleTest` support from the CMake build system 
for the MATLAB interface.
   
   As was [the case for the Python 
bindings](https://github.com/apache/arrow/pull/14117), the cost of maintenance 
for `GoogleTest` support in the MATLAB interface infrastructure is high without 
providing much value at this time.
   
   Rather than relying on writing C++ tests directly using `GoogleTest`, we 
believe that it would be easier to just call a MEX function from MATLAB to test 
C++ code where needed.
   
   On a somewhat related note - removing `GoogleTest` support will help unblock 
https://github.com/apache/arrow/pull/37773 as discussed in 
https://github.com/apache/arrow/pull/37773#issuecomment-1724207859.
   
   ### Rationale for this change
   
   1. `GoogleTest` support adds a lot of additional complexity to the CMake 
build system for the MATLAB interface, and we currently don't have any 
standalone C++ tests for the MATLAB interface code.
   2. In order to use `GoogleTest` in the MATLAB CI workflows, we are currently 
relying on building the tests for the Arrow C++ libraries in order to "re-use" 
the `GoogleTest binaries. This adds additional overhead to the MATLAB CI 
workflows.
   3. If we want to test some internal C++ code for the MATLAB interface in the 
future, we can instead use a MEX function to call the code from a MATLAB test 
as suggested by @kou in 
https://github.com/apache/arrow/issues/37532#issue-1879320217.
   4. There is [precedent for testing internal C++ code without GoogleTest for 
the Python bindings](https://github.com/apache/arrow/pull/14117).
   
   ### What changes are included in this PR?
   
   1. Removed the `MATLAB_BUILD_TESTS` flag from the CMake build system for the 
MATLAB interface since there are no longer any C++ tests for the MATLAB 
interface to build.
   2. Updated the `matlab_build.sh` CI workflow script to avoid building the 
tests for the Arrow C++ libraries and to no longer call `ctest`.
   3. Updated the `README.md` for the MATLAB interface to no longer mention 
building or running C++ tests.
   6. Updated the design document for the MATLAB Interface to no longer mention 
`GoogleTest` since we may end up testing internal C++ code using MEX function 
calls from MATLAB instead.
   
   ### Are there any user-facing changes?
   
   Yes.
   
   There are no longer any C++ tests for the MATLAB interface. The 
`MATLAB_BUILD_TESTS` flag has been removed from the CMake build system to 
reflect this change. If a user supplies a value for `MATLAB_BUILD_TESTS` when 
building the MATLAB interface, the flag will be ignored by CMake.
   
   ### Future Directions
   
   1. Add more developer-focused documentation on how to test C++ code via MEX 
function calls from MATLAB.
   
   ### Notes
   
   1. In the future, we can consider testing internal C++ code using MEX 
function calls from MATLAB tests as suggested by @kou in 
https://github.com/apache/arrow/issues/37532#issue-1879320217. Currently, we 
don't have any C++ tests that need to be adapted to use this approach.
   2. Thank you @sgilmore10 for your help with this pull request!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to