Skip to content
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

Temporarily Disable Java Integration Tests #4957

Merged
merged 3 commits into from
Oct 20, 2023

Conversation

tustvold
Copy link
Contributor

Which issue does this PR close?

Closes #.

Rationale for this change

Seeing failures on master, this might fix it

https://github.com/apache/arrow-rs/actions/runs/6574785367/job/17860535457

What changes are included in this PR?

Are there any user-facing changes?

@tustvold tustvold force-pushed the integration-test-dependencies branch from a6f87f7 to cab4411 Compare October 19, 2023 13:38
@tustvold tustvold added the development-process Related to development process of arrow-rs label Oct 19, 2023
@tustvold
Copy link
Contributor Author

@pitrou Any ideas, this at least appears to coincide with apache/arrow#38248

@pitrou
Copy link
Member

pitrou commented Oct 19, 2023

@tustvold
Copy link
Contributor Author

tustvold commented Oct 19, 2023

Is there perhaps some way that we can script these in a way that can be reused by arrow-rs as opposed to hard-coded in a docker-compose file, that we keep having to incorporate changes from as things change?

@pitrou
Copy link
Member

pitrou commented Oct 19, 2023

That's a good question. I'm not sure how to do that as these environment variables are influencing build steps for various implementations. Perhaps we need a single overarching environment variable such as ARROW_INTEGRATION_ONLY=1. @kou Any idea?

@kou
Copy link
Member

kou commented Oct 19, 2023

How about adding ci/scripts/integration_build.sh to apache/arrow and use it in apache/arrow's docker-compose.yml and apache/arrow-rs's .github/workflows/integration.yml?

diff --git a/docker-compose.yml b/docker-compose.yml
index e54c609e5..6b5b777c9 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -1734,18 +1734,8 @@ services:
       # Tell Archery where the arrow C++ binaries are located
       ARROW_CPP_EXE_PATH: /build/cpp/debug
       ARROW_GO_INTEGRATION: 1
-      ARROW_JAVA_CDATA: "ON"
-      JAVA_JNI_CMAKE_ARGS: >-
-        -DARROW_JAVA_JNI_ENABLE_DEFAULT=OFF
-        -DARROW_JAVA_JNI_ENABLE_C=ON
     command:
-      ["/arrow/ci/scripts/rust_build.sh /arrow /build &&
-        /arrow/ci/scripts/cpp_build.sh /arrow /build &&
-        /arrow/ci/scripts/csharp_build.sh /arrow /build &&
-        /arrow/ci/scripts/go_build.sh /arrow &&
-        /arrow/ci/scripts/java_jni_build.sh /arrow $${ARROW_HOME} /build /tmp/dist/java/$$(arch) &&
-        /arrow/ci/scripts/java_build.sh /arrow /build /tmp/dist/java &&
-        /arrow/ci/scripts/js_build.sh /arrow /build &&
+      ["/arrow/ci/scripts/integration_build.sh /arrow /build &&
         /arrow/ci/scripts/integration_arrow.sh /arrow /build"]
 
   ################################ Docs #######################################
diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml
index 62d2d2cb1a..0c3e4213fd 100644
--- a/.github/workflows/integration.yml
+++ b/.github/workflows/integration.yml
@@ -88,42 +88,10 @@ jobs:
         with:
           path: rust
           fetch-depth: 0
-      - name: Install pythonnet
-        run: conda run --no-capture-output pip install pythonnet
-      - name: Install archery
-        run: conda run --no-capture-output pip install -e dev/archery[integration]
-      - name: Make build directory
-        run: mkdir /build
-      - name: Build Rust
-        run: conda run --no-capture-output ci/scripts/rust_build.sh $PWD /build
-      - name: Build C++
-        run: conda run --no-capture-output ci/scripts/cpp_build.sh $PWD /build
-      - name: Build C#
-        run: conda run --no-capture-output ci/scripts/csharp_build.sh $PWD /build
-      - name: Build Go
-        run: conda run --no-capture-output ci/scripts/go_build.sh $PWD
-      - name: Build Java
-        run: conda run --no-capture-output ci/scripts/java_build.sh $PWD /build
-      - name: Build JS
-        run: conda run --no-capture-output ci/scripts/js_build.sh $PWD /build
+      - name: Build
+        run: conda run --no-capture-output ci/scripts/integration_build.sh $PWD /build
       - name: Run integration tests
-        run: |
-          conda run --no-capture-output archery integration \
-            --run-flight \
-            --run-c-data \
-            --run-ipc \
-            --with-cpp=1 \
-            --with-csharp=1 \
-            --with-java=1 \
-            --with-js=1 \
-            --with-go=1 \
-            --with-rust=1 \
-            --gold-dirs=testing/data/arrow-ipc-stream/integration/0.14.1 \
-            --gold-dirs=testing/data/arrow-ipc-stream/integration/0.17.1 \
-            --gold-dirs=testing/data/arrow-ipc-stream/integration/1.0.0-bigendian \
-            --gold-dirs=testing/data/arrow-ipc-stream/integration/1.0.0-littleendian \
-            --gold-dirs=testing/data/arrow-ipc-stream/integration/2.0.0-compression \
-            --gold-dirs=testing/data/arrow-ipc-stream/integration/4.0.0-shareddict
+        run: conda run --no-capture-output ci/scripts/integration_arrow.sh $PWD /build
 
   # test FFI against the C-Data interface exposed by pyarrow
   pyarrow-integration-test:

@tustvold
Copy link
Contributor Author

Something like that would be perfect. One small comment is it it would be good if we preserved the ability to override the tested languages, as in the past this has been very useful for temporarily disabling certain languages when they are having issues

@kou
Copy link
Member

kou commented Oct 20, 2023

It makes sense. We can implement it by environment variables such as ARROW_INTEGRATION_${LANG}=1. We use the environment variable approach in our verification script: https://github.com/apache/arrow/blob/a0e58f18dc643f9bfd4efa32331ddf477643fed2/dev/release/verify-release-candidate.sh#L1160-L1191

@tustvold
Copy link
Contributor Author

tustvold commented Oct 20, 2023

I've opted to temporarily disable the Java tests in order to get master green, and filed #4963 to track a better solution

@tustvold tustvold changed the title Add additional integration test dependencies Temporarily Disable Java Integration Tests Oct 20, 2023
@tustvold tustvold merged commit 0b9105d into apache:master Oct 20, 2023
31 checks passed
kou pushed a commit to apache/arrow that referenced this pull request Oct 25, 2023
…esting (#38403)

### Rationale for this change

It is currently cumbersome to run integration tests from an other repository, as each tested implementation must be built explicitly.

See explanatory comment at apache/arrow-rs#4957 (comment)

### What changes are included in this PR?

Provide a wrapper script to automate building the implementations being tested.

Allow usage of environment variables such as `ARROW_INTEGRATION_CPP`, etc. to selectively disable implementations.

### Are these changes tested?

Yes, by construction.

### Are there any user-facing changes?

No.
* Closes: #38402

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
JerAguilon pushed a commit to JerAguilon/arrow that referenced this pull request Oct 25, 2023
…tion testing (apache#38403)

### Rationale for this change

It is currently cumbersome to run integration tests from an other repository, as each tested implementation must be built explicitly.

See explanatory comment at apache/arrow-rs#4957 (comment)

### What changes are included in this PR?

Provide a wrapper script to automate building the implementations being tested.

Allow usage of environment variables such as `ARROW_INTEGRATION_CPP`, etc. to selectively disable implementations.

### Are these changes tested?

Yes, by construction.

### Are there any user-facing changes?

No.
* Closes: apache#38402

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
alamb pushed a commit to alamb/arrow-rs that referenced this pull request Nov 8, 2023
* Add additional integration test dependencies

* Temporarily disable Java

* Remove jpype
alamb added a commit that referenced this pull request Nov 8, 2023
* Temporarily Disable Java Integration Tests (#4957)

* Add additional integration test dependencies

* Temporarily disable Java

* Remove jpype

* Use new integration scripts (#4963) (#4988)

---------

Co-authored-by: Raphael Taylor-Davies <[email protected]>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…tion testing (apache#38403)

### Rationale for this change

It is currently cumbersome to run integration tests from an other repository, as each tested implementation must be built explicitly.

See explanatory comment at apache/arrow-rs#4957 (comment)

### What changes are included in this PR?

Provide a wrapper script to automate building the implementations being tested.

Allow usage of environment variables such as `ARROW_INTEGRATION_CPP`, etc. to selectively disable implementations.

### Are these changes tested?

Yes, by construction.

### Are there any user-facing changes?

No.
* Closes: apache#38402

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…tion testing (apache#38403)

### Rationale for this change

It is currently cumbersome to run integration tests from an other repository, as each tested implementation must be built explicitly.

See explanatory comment at apache/arrow-rs#4957 (comment)

### What changes are included in this PR?

Provide a wrapper script to automate building the implementations being tested.

Allow usage of environment variables such as `ARROW_INTEGRATION_CPP`, etc. to selectively disable implementations.

### Are these changes tested?

Yes, by construction.

### Are there any user-facing changes?

No.
* Closes: apache#38402

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development-process Related to development process of arrow-rs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants