Skip to content

chore(dev/release): fix verification issues - #4937

Merged
lidavidm merged 4 commits into
apache:mainfrom
lidavidm:verification
Oct 22, 2026
Merged

lidavidm merged 4 commits into
apache:mainfrom
lidavidm:verification

Conversation

@lidavidm

@lidavidm lidavidm commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Assisted-by: GPT-6 Astra

@lidavidm
lidavidm marked this pull request as ready for review October 21, 2026 04:24
@lidavidm
lidavidm requested a review from kou as a code owner October 21, 2026 04:24
@lidavidm

Copy link
Copy Markdown
Member Author

@amoeba amoeba left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

adbc_*-cp${pyver/.}-cp${python/.}-${platform}*${arch}*.whl \
adbc_*-py3-none-${platform}*${arch}*.whl
${ADBC_DIR}/ci/scripts/python_wheel_unix_test.sh ${ADBC_DIR}
unset PYTHON_GIL

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems out of place but safe.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it turns out that the Conda environment activate/deactivate was barfing if it was set.

@lidavidm
lidavidm merged commit e1795cf into apache:main Oct 22, 2026
96 checks passed
Comment on lines +969 to +976
local test_arguments=()
if [[ "$(uname)" == "Darwin" && "$(uname -m)" == "x86_64" ]]; then
show_info "Skipping testDriverJni: JNI binaries are not available for macOS/x86_64"
test_arguments+=('-Dtest=!VerifyReleaseCandidateTest#testDriverJni')
fi

mvn "${maven_arguments[@]}" test \
"${test_arguments[@]}" \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be cleaner if we put this in the Java test?

diff --git a/dev/release/verify-release-candidate.sh b/dev/release/verify-release-candidate.sh
index 3c8e123f2..c59fa0fdc 100755
--- a/dev/release/verify-release-candidate.sh
+++ b/dev/release/verify-release-candidate.sh
@@ -966,14 +966,7 @@ print(version.text)
         "-Djavadoc=${javadoc_path}"
   done
 
-  local test_arguments=()
-  if [[ "$(uname)" == "Darwin" && "$(uname -m)" == "x86_64" ]]; then
-    show_info "Skipping testDriverJni: JNI binaries are not available for macOS/x86_64"
-    test_arguments+=('-Dtest=!VerifyReleaseCandidateTest#testDriverJni')
-  fi
-
   mvn "${maven_arguments[@]}" test \
-      "${test_arguments[@]}" \
       "-Dadbc.version=${java_version}" \
       -f "${SOURCE_DIR}/verify/java/pom.xml"
 }
diff --git a/dev/release/verify/java/src/test/java/org/apache/arrow/adbc/verify/VerifyReleaseCandidateTest.java b/dev/release/verify/java/src/test/java/org/apache/arrow/adbc/verify/VerifyReleaseCandidateTest.java
index 83f6d5512..03c4abfcc 100644
--- a/dev/release/verify/java/src/test/java/org/apache/arrow/adbc/verify/VerifyReleaseCandidateTest.java
+++ b/dev/release/verify/java/src/test/java/org/apache/arrow/adbc/verify/VerifyReleaseCandidateTest.java
@@ -85,6 +85,17 @@ class VerifyReleaseCandidateTest {
   @Test
   void testDriverJni() {
     var driver = new JniDriver(allocator);
+    if (OS.MAC.isCurrentOs() && "x86_64".equalsIgnoreCase(System.getProperty("os.arch"))) {
+      // JNI binaries are not available for macOS x86_64.
+      ExceptionInInitializerError exception =
+          assertThrows(
+              ExceptionInInitializerError.class, () -> driver.load().driver("nonexistent").open());
+      assertThat(exception)
+          .hasRootCauseInstanceOf(RuntimeException.class)
+          .hasRootCauseMessage("ADBC JNI driver not supported on architecture x86_64");
+      return;
+    }
+
     if (OS.WINDOWS.isCurrentOs() && "aarch64".equalsIgnoreCase(System.getProperty("os.arch"))) {
       // JNI binaries are not available for Windows ARM64.
       ExceptionInInitializerError exception =

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would, but then I'd have to cut the release from scratch

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Want to file an issue and we can get this fixed for the next release?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, of course

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants