ARROW-4945: [Flight] Enable integration tests in Travis#4003
Conversation
|
I think it's worth adding this as a new entry in the matrix build as grpc/flight can be big component by themselves (and potentially adding skip feature). |
|
Sorry, this dropped off my radar, but I'll back on this soon - thanks for the suggestion! |
integration/integration_test.py
Outdated
There was a problem hiding this comment.
Hmm, why? We should fix the failure instead...
There was a problem hiding this comment.
It's not supported yet: https://issues.apache.org/jira/browse/ARROW-3200
There was a problem hiding this comment.
Then perhaps simply skip it until it's supported? The "expected fail" machinery doesn't look very useful to me.
|
Alright, the integration tests now pass in Travis, though the OSX build flaked. I can disable the Flight integration entry if it's too much of a burden for now. (Well, I guess that defeats the point of this.) |
.travis.yml
Outdated
There was a problem hiding this comment.
I'm curious: what do we gain by doing this in a separate matrix entry?
There was a problem hiding this comment.
@pitrou my rationale is that this can be a big/long check due to grpc dependency and the use of integration tests. If we start adding multi-lang tests, like the ipc-json suite, than I think it's worth adding a seperated entry (for parallelism).
There was a problem hiding this comment.
But if the Flight integration entry is doing a superset of what the non-Flight integration entry does, we're not winning anything (and we're probably losing parallelism credits).
There was a problem hiding this comment.
I could change it to just run the Flight tests. We'd still pay the cost of setting up and building everything though.
There was a problem hiding this comment.
How long does it take to run all integration tests in a single job? My guess is that we're better off with a single job. Especially as we're hitting Travis-CI job queueing issues nowadays.
There was a problem hiding this comment.
28 minutes, vs 24 for non-Flight tests. Ok, so I'll re-consolidate them, it doesn't seem to be much overhead (for now).
Codecov Report
@@ Coverage Diff @@
## master #4003 +/- ##
=========================================
Coverage ? 87.83%
=========================================
Files ? 738
Lines ? 90851
Branches ? 1252
=========================================
Hits ? 79797
Misses ? 10937
Partials ? 117Continue to review full report at Codecov.
|
I encounter the following build error when I built the tarball of 0.13.0 RC. ``` [38/597] Performing configure step for 'grpc_ep' FAILED: grpc_ep-prefix/src/grpc_ep-stamp/grpc_ep-configure cd /Users/mrkn/Downloads/apache-arrow-0.13.0/cpp/build/grpc_ep-prefix/src/grpc_ep-build && /opt/brew/Cellar/cmake/3.13.3/bin/cmake -P /Users/mrkn/Downloads/apache-arrow-0.13.0/cpp/build/grpc_ep-prefix/src/grpc_ep-stamp/grpc_ep-configure-RELEASE.cmake && /opt/brew/Cellar/c make/3.13.3/bin/cmake -E touch /Users/mrkn/Downloads/apache-arrow-0.13.0/cpp/build/grpc_ep-prefix/src/grpc_ep-stamp/grpc_ep-configure CMake Error at /Users/mrkn/Downloads/apache-arrow-0.13.0/cpp/build/grpc_ep-prefix/src/grpc_ep-stamp/grpc_ep-configure-RELEASE.cmake:16 (message): Command failed: 1 '/opt/brew/Cellar/cmake/3.13.3/bin/cmake' '-DCMAKE_BUILD_TYPE=RELEASE' '-DCMAKE_PREFIX_PATH=';/opt/brew;/Users/mrkn/src/github.com/apache/arrow/cpp/build/gflags_ep-prefix/src/gflags_ep;/Users/mrkn/Downloads/apache-arrow-0.13.0/cpp/thirdparty/cares_ep-install;'' '-DgRPC _CARES_PROVIDER=package' '-DgRPC_GFLAGS_PROVIDER=package' '-DgRPC_PROTOBUF_PROVIDER=package' '-DgRPC_SSL_PROVIDER=package' '-DgRPC_ZLIB_PROVIDER=package' '-DCMAKE_CXX_FLAGS= -Qunused-arguments -fcolor-diagnostics -O3 -DNDEBUG -O3 -DNDEBUG -fPIC' '-DCMAKE_C_FLAGS= -Qunused-arguments -O3 -DNDEBUG -O3 -DNDEBUG -fPIC' '-DCMAKE_INSTALL_PREFIX=/Users/mrkn/Downloads/apache-arrow-0.13.0/cpp/thirdparty/grpc_ep-install' '-DCMAKE_INSTALL_LIBDIR=lib' '-DProtobuf_PROTOC_LIBRARY=/opt/brew/lib/libprotoc.dylib' '-DBUILD_SHARED_LIBS=OFF' '-GNinja' '/Users/mrkn/Downloads/apache-arrow-0.13.0/cpp/build/grpc_ep-prefix/src/grpc_ep' See also /Users/mrkn/Downloads/apache-arrow-0.13.0/cpp/build/grpc_ep-prefix/src/grpc_ep-stamp/grpc_ep-configure-*.log ``` grpc_ep's build log is given below: ``` $ cat /Users/mrkn/Downloads/apache-arrow-0.13.0/cpp/build/grpc_ep-prefix/src/grpc_ep-stamp/grpc_ep-configure-*.log CMake Error at cmake/cares.cmake:38 (find_package): Could not find a package configuration file provided by "c-ares" with any of the following names: c-aresConfig.cmake c-ares-config.cmake Add the installation prefix of "c-ares" to CMAKE_PREFIX_PATH or set "c-ares_DIR" to a directory containing one of the above files. If "c-ares" provides a separate development package or SDK, be sure it has been installed. Call Stack (most recent call first): CMakeLists.txt:139 (include) (snip) ``` This build log says that grpc_ep couldn't find cares_ep. In this case, grpc_ep was built before cares_ep. This pull-request could fix this dependency error on my environment. Author: Kenta Murata <mrkn@mrkn.jp> Closes #4064 from mrkn/cpp_grpc_depends_on_cares and squashes the following commits: 1a37054 <Kenta Murata> Specify dependencies of grpc_ep
integration/integration_test.py
Outdated
There was a problem hiding this comment.
Ok, sorry to come back to this, but why do you need both JsonFile and TestCase? They seem to represent the same thing...
There was a problem hiding this comment.
No worries. It's because of get_static_json_files below. We could read the existing JSON into Python and put it into a JsonFile, but that seems like a waste. Or, I suppose, JsonFile could handle both cases (generated and static data) - I can make that change.
6 participants
No description provided.