You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2023/09/04 00:57:07 UTC

[GitHub] [arrow] kou opened a new issue, #37532: [MATLAB] Reconsider GoogleTest needs

kou opened a new issue, #37532:
URL: https://github.com/apache/arrow/issues/37532

   ### Describe the enhancement requested
   
   Now, our build system has GoogleTest support but it's not used. There is only a placeholder test that tests nothing.
   We had one test for `mex_util` but it removed by GH-37204.
   We use the MATLAB's testing framework for all tests.
   
   The GoogleTest support needs non-trivial CMake codes. If we can drop the GoogleTest support, we can reduce maintenance cost for it and reduce build time of the MATLAB bindings.
   
   But we have the following use-case of the GoogleTest support:
   
   https://github.com/apache/arrow/pull/37483#issuecomment-1702912785
   
   > I think we want to keep the GoogleTest support for now because we do plan on adding C++ unit tests. There are a few error-conditions that cannot be triggered from MATLAB for which we want to add C++ unit tests.
   
   Can we use another approach for the use-case?
   
   For example, can we write a C++ method that causes an error-condition and register the method to MATLAB?
   
   ```diff
   diff --git a/matlab/src/cpp/arrow/matlab/array/proxy/array.cc b/matlab/src/cpp/arrow/matlab/array/proxy/array.cc
   index 9dc14c341..cedb58793 100644
   --- a/matlab/src/cpp/arrow/matlab/array/proxy/array.cc
   +++ b/matlab/src/cpp/arrow/matlab/array/proxy/array.cc
   @@ -37,6 +37,9 @@ namespace arrow::matlab::array::proxy {
            REGISTER_METHOD(Array, type);
            REGISTER_METHOD(Array, isEqual);
    
   +#ifdef ARROW_MATLAB_TEST
   +        REGISTER_METHOD(Array, errorA);
   +#endif
        }
    
        std::shared_ptr<arrow::Array> Array::unwrap() {
   @@ -115,4 +118,11 @@ namespace arrow::matlab::array::proxy {
            mda::ArrayFactory factory;
            context.outputs[0] = factory.createScalar(is_equal);
        }
   +
   +#ifdef ARROW_MATLAB_TEST
   +    void Array::errorA(libmexclass::proxy::method::Context& context) {
   +        auto errorCondition = arrow::Status::Invalid("error A!");
   +        MATLAB_ERROR_IF_NOT_OK_WITH_CONTEXT(errorCondition, context, ...ERROR_ID);
   +    }
   +#endif
    }
   ```
   
   If we can call the method from MATLAB, can we assert it by the MATLAB testing framework?
   
   FYI: We removed GoogleTest dependency from PyArrow by GH-14117/GH-32328 because maintaining GoogleTest support in `python/` was high cost for us.
   only when 
   
   ### Component(s)
   
   MATLAB


-- 
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: issues-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kevingurney commented on issue #37532: [CI][Docs][MATLAB] Remove `GoogleTest` support from the CMake build system for the MATLAB interface

Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on issue #37532:
URL: https://github.com/apache/arrow/issues/37532#issuecomment-1724302533

   1. Re-worded title of this issue.
   2. Added labels `Component: Documentation` and `Component: Continuous Integration` since this impacts the documentation and MATLAB CI workflows.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kevingurney closed issue #37532: [CI][Docs][MATLAB] Remove `GoogleTest` support from the CMake build system for the MATLAB interface

Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney closed issue #37532: [CI][Docs][MATLAB] Remove `GoogleTest` support from the CMake build system for the MATLAB interface
URL: https://github.com/apache/arrow/issues/37532


-- 
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: issues-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org