You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/10/21 22:20:51 UTC
(datafusion) branch gh-readonly-queue/main/pr-26564-659b4758f8ebb5eb6fdee95f08abf08b77e7c2ab created (now 0eafd4de89)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-26564-659b4758f8ebb5eb6fdee95f08abf08b77e7c2ab
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at 0eafd4de89 chore: move datafusion-ffi integration tests into their own crate (#26564)
This branch includes the following new commits:
new 0eafd4de89 chore: move datafusion-ffi integration tests into their own crate (#26564)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: chore: move datafusion-ffi integration tests into their own crate (#26564)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-26564-659b4758f8ebb5eb6fdee95f08abf08b77e7c2ab
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 0eafd4de89388d6651dfd07c9b4ac74bac119e0d
Author: Jan Procházka <ho...@gmail.com>
AuthorDate: Wed Oct 21 22:03:52 2026 +0000
chore: move datafusion-ffi integration tests into their own crate (#26564)
## Which issue does this PR close?
- Closes #26559.
## Rationale for this change
`datafusion-ffi` is built as a `cdylib` only so that its own integration
tests can `dlopen` it.
Every downstream build pays for compiling and linking that unused shared
library, and `sccache` can't cache crates with a `cdylib` crate type.
## What changes are included in this PR?
- New unpublished crate `datafusion-ffi-integration-tests` in
`datafusion/ffi/integration-tests/`, built as a `cdylib`, which
re-exports the `#[no_mangle]` test entry points from
`datafusion_ffi::tests`.
Its `integration-tests` feature enables
`datafusion-ffi/integration-tests`, so regular workspace builds still
don't compile the test module.
- `datafusion/ffi/tests/` moves there unchanged, and `find_library`
looks for the new library name.
- `datafusion-ffi` is now a plain `rlib`.
- The `test ffi` xtask step and the `datafusion-ffi` agent skill include
the new crate.
The new crate needs both `cdylib` and `rlib`: with only `cdylib`, Cargo
doesn't build the library before running the crate's integration tests.
## What is the testing strategy for this PR?
Covered by the existing FFI integration tests, which now load the new
crate's `cdylib`.
## Are there any user-facing changes?
The public API and ABI of `datafusion-ffi` are unchanged. Crates that
build their own `cdylib` on top of `datafusion-ffi` are unaffected.
---
This PR was prepared with the help of a coding agent (Claude).
Co-authored-by: Tim Saucer <ti...@gmail.com>
---
.ai/skills/datafusion-ffi/SKILL.md | 10 ++---
Cargo.lock | 20 +++++++++
Cargo.toml | 1 +
datafusion/ffi/Cargo.toml | 1 -
datafusion/ffi/integration-tests/Cargo.toml | 51 ++++++++++++++++++++++
.../cases/mod.rs => integration-tests/src/lib.rs} | 13 ++----
.../{ => integration-tests}/tests/cases/catalog.rs | 0
.../{ => integration-tests}/tests/cases/config.rs | 0
.../tests/cases/execution_plan.rs | 0
.../ffi/{ => integration-tests}/tests/cases/mod.rs | 0
.../tests/cases/physical_optimizer.rs | 0
.../tests/cases/query_planner.rs | 0
.../{ => integration-tests}/tests/cases/udaf.rs | 0
.../ffi/{ => integration-tests}/tests/cases/udf.rs | 0
.../{ => integration-tests}/tests/cases/udtf.rs | 0
.../{ => integration-tests}/tests/cases/udwf.rs | 0
.../tests/ffi_integration.rs | 0
.../ffi/{ => integration-tests}/tests/utils/mod.rs | 0
datafusion/ffi/src/tests/utils.rs | 2 +-
xtask/src/ci_steps.rs | 3 ++
20 files changed, 85 insertions(+), 16 deletions(-)
diff --git a/.ai/skills/datafusion-ffi/SKILL.md b/.ai/skills/datafusion-ffi/SKILL.md
index ba02d22c09..88f13abf47 100644
--- a/.ai/skills/datafusion-ffi/SKILL.md
+++ b/.ai/skills/datafusion-ffi/SKILL.md
@@ -197,21 +197,21 @@ Templates: `test_ffi_table_provider_local_bypass` and `test_round_trip_ffi_table
What unit tests catch: Rust-level correctness (translation logic, lifetime bugs, leaks under valgrind/miri, Send/Sync, error propagation, codec round-trips). What they **cannot** catch: real ABI bugs. Both producer and consumer share `#[repr(C)]` layout because they are the exact same struct definition in memory.
-#### b. Cross-library integration tests (`tests/ffi_*.rs`, gated by the `integration-tests` feature)
+#### b. Cross-library integration tests (`integration-tests/tests/`, gated by the `integration-tests` feature)
-The crate is published as `crate-type = ["cdylib", "rlib"]`. The integration tests in `datafusion/ffi/tests/` use `libloading` to `dlopen` the crate's own `cdylib` and call `datafusion_ffi_get_module` — a `#[unsafe(no_mangle)] extern "C"` entry point defined in `src/tests/mod.rs` and gated by `#[cfg(feature = "integration-tests")]`. The test executable links against the rlib (consumer side); the dlopen'd cdylib is the producer side. Even though both are built from the same source, they a [...]
+The integration tests live in the unpublished `datafusion-ffi-integration-tests` crate in `datafusion/ffi/integration-tests/`, so that `datafusion-ffi` itself is a plain rlib. That crate is built as a `cdylib` that exports `datafusion_ffi_get_module` — a `#[unsafe(no_mangle)] extern "C"` entry point defined in `src/tests/mod.rs` and gated by `#[cfg(feature = "integration-tests")]`. Its tests use `libloading` to `dlopen` that `cdylib`. The test executable links against the `datafusion-ffi [...]
Run with:
```bash
-cargo test -p datafusion-ffi --features integration-tests
+cargo test -p datafusion-ffi -p datafusion-ffi-integration-tests --features integration-tests
```
To add coverage for a new wrapper:
1. **Add a constructor** in `src/tests/<area>.rs` (or a new file there). Return a populated `FFI_X` from a known-good native type.
2. **Wire it into `ForeignLibraryModule`** in `src/tests/mod.rs`: add a field of type `extern "C" fn(...) -> FFI_X` and populate it in `datafusion_ffi_get_module`. This struct is the cross-library contract. Adding a field is itself an ABI change for the test module; integration tests will rebuild the cdylib automatically.
-3. **Add the test** in `tests/ffi_<area>.rs` under `#[cfg(feature = "integration-tests")] mod tests { … }`. Call `datafusion_ffi::tests::utils::get_module()` to load the cdylib, invoke your constructor through the returned `ForeignLibraryModule`, convert into `Arc<dyn X>`, and exercise every method.
+3. **Add the test** in `integration-tests/tests/cases/<area>.rs`, registered in `cases/mod.rs`, under `mod tests { … }`. Call `datafusion_ffi::tests::utils::get_module()` to load the cdylib, invoke your constructor through the returned `ForeignLibraryModule`, convert into `Arc<dyn X>`, and exercise every method.
When adding a method to an existing wrapper, reuse an existing fixture and constructor when a small trait-method override can cover it. This applies both to omitted default methods and methods newly added to the trait. Add a dedicated test type or `ForeignLibraryModule` field only when the existing fixtures cannot cover the method.
@@ -347,7 +347,7 @@ When reviewing a PR that touches `datafusion/ffi/`:
7. **Stabby types** for strings/vecs; crate-local `FFI_Option`/`FFI_Result` for optional/fallible payloads?
8. **Async** uses `FfiFuture` + `.into_ffi()`, never blocking?
9. **Codec** present on any method that ships an `Expr` / plan?
-10. **Unit tests** include both local-bypass and `mock_foreign_marker_id` forced-foreign cases? **Integration tests** in `tests/ffi_*.rs` exist for any wrapper that takes/returns non-trivial FFI types, and for *every* layout change? `cargo test -p datafusion-ffi --features integration-tests` must pass before merging an ABI-affecting PR.
+10. **Unit tests** include both local-bypass and `mock_foreign_marker_id` forced-foreign cases? **Integration tests** in `integration-tests/tests/` exist for any wrapper that takes/returns non-trivial FFI types, and for *every* layout change? `cargo test -p datafusion-ffi -p datafusion-ffi-integration-tests --features integration-tests` must pass before merging an ABI-affecting PR.
11. **No `datafusion` runtime dep** crept into `Cargo.toml`?
12. **`Arc::clone(&x)`** everywhere — no implicit `x.clone()` on `Arc` (the lint will reject it but worth pre-flagging).
13. **`cargo clippy --all-targets --all-features -- -D warnings`** clean on the crate?
diff --git a/Cargo.lock b/Cargo.lock
index 714fc492fb..5df784c053 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2268,6 +2268,26 @@ dependencies = [
"tokio",
]
+[[package]]
+name = "datafusion-ffi-integration-tests"
+version = "0.1.0"
+dependencies = [
+ "arrow",
+ "arrow-schema",
+ "datafusion",
+ "datafusion-catalog",
+ "datafusion-common",
+ "datafusion-execution",
+ "datafusion-expr",
+ "datafusion-ffi",
+ "datafusion-physical-expr-common",
+ "datafusion-physical-optimizer",
+ "datafusion-physical-plan",
+ "datafusion-proto",
+ "datafusion-session",
+ "tokio",
+]
+
[[package]]
name = "datafusion-functions"
version = "55.1.0"
diff --git a/Cargo.toml b/Cargo.toml
index f3cc9aed97..e9ec33e5d0 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -32,6 +32,7 @@ members = [
"datafusion/expr-common",
"datafusion/execution",
"datafusion/ffi",
+ "datafusion/ffi/integration-tests",
"datafusion/functions",
"datafusion/functions-aggregate",
"datafusion/functions-aggregate-common",
diff --git a/datafusion/ffi/Cargo.toml b/datafusion/ffi/Cargo.toml
index affcff3dbd..c0b141ed7f 100644
--- a/datafusion/ffi/Cargo.toml
+++ b/datafusion/ffi/Cargo.toml
@@ -38,7 +38,6 @@ workspace = true
[lib]
name = "datafusion_ffi"
-crate-type = ["cdylib", "rlib"]
# Note to developers: do *not* add `datafusion` as a dependency in this crate.
# It increases build times and library binary size for users.
diff --git a/datafusion/ffi/integration-tests/Cargo.toml b/datafusion/ffi/integration-tests/Cargo.toml
new file mode 100644
index 0000000000..7edd5215c5
--- /dev/null
+++ b/datafusion/ffi/integration-tests/Cargo.toml
@@ -0,0 +1,51 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[package]
+name = "datafusion-ffi-integration-tests"
+description = "Cross-library integration tests for datafusion-ffi"
+version = "0.1.0"
+edition = { workspace = true }
+license = { workspace = true }
+publish = false
+
+[lints]
+workspace = true
+
+[lib]
+crate-type = ["cdylib", "rlib"]
+
+[features]
+integration-tests = ["datafusion-ffi/integration-tests"]
+
+[dependencies]
+datafusion-ffi = { workspace = true }
+
+[dev-dependencies]
+arrow = { workspace = true }
+arrow-schema = { workspace = true }
+datafusion = { workspace = true, default-features = false, features = ["sql"] }
+datafusion-catalog = { workspace = true }
+datafusion-common = { workspace = true }
+datafusion-execution = { workspace = true }
+datafusion-expr = { workspace = true }
+datafusion-physical-expr-common = { workspace = true }
+datafusion-physical-optimizer = { workspace = true }
+datafusion-physical-plan = { workspace = true }
+datafusion-proto = { workspace = true, default-features = false }
+datafusion-session = { workspace = true }
+tokio = { workspace = true }
diff --git a/datafusion/ffi/tests/cases/mod.rs b/datafusion/ffi/integration-tests/src/lib.rs
similarity index 83%
copy from datafusion/ffi/tests/cases/mod.rs
copy to datafusion/ffi/integration-tests/src/lib.rs
index b32a54af49..6457ff18c3 100644
--- a/datafusion/ffi/tests/cases/mod.rs
+++ b/datafusion/ffi/integration-tests/src/lib.rs
@@ -15,12 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-mod catalog;
-mod config;
-mod execution_plan;
-mod physical_optimizer;
-mod query_planner;
-mod udaf;
-mod udf;
-mod udtf;
-mod udwf;
+#[cfg(feature = "integration-tests")]
+pub use datafusion_ffi::tests::{
+ datafusion_ffi_get_module, datafusion_ffi_test_create_exec_with_byte_metrics,
+};
diff --git a/datafusion/ffi/tests/cases/catalog.rs b/datafusion/ffi/integration-tests/tests/cases/catalog.rs
similarity index 100%
rename from datafusion/ffi/tests/cases/catalog.rs
rename to datafusion/ffi/integration-tests/tests/cases/catalog.rs
diff --git a/datafusion/ffi/tests/cases/config.rs b/datafusion/ffi/integration-tests/tests/cases/config.rs
similarity index 100%
rename from datafusion/ffi/tests/cases/config.rs
rename to datafusion/ffi/integration-tests/tests/cases/config.rs
diff --git a/datafusion/ffi/tests/cases/execution_plan.rs b/datafusion/ffi/integration-tests/tests/cases/execution_plan.rs
similarity index 100%
rename from datafusion/ffi/tests/cases/execution_plan.rs
rename to datafusion/ffi/integration-tests/tests/cases/execution_plan.rs
diff --git a/datafusion/ffi/tests/cases/mod.rs b/datafusion/ffi/integration-tests/tests/cases/mod.rs
similarity index 100%
rename from datafusion/ffi/tests/cases/mod.rs
rename to datafusion/ffi/integration-tests/tests/cases/mod.rs
diff --git a/datafusion/ffi/tests/cases/physical_optimizer.rs b/datafusion/ffi/integration-tests/tests/cases/physical_optimizer.rs
similarity index 100%
rename from datafusion/ffi/tests/cases/physical_optimizer.rs
rename to datafusion/ffi/integration-tests/tests/cases/physical_optimizer.rs
diff --git a/datafusion/ffi/tests/cases/query_planner.rs b/datafusion/ffi/integration-tests/tests/cases/query_planner.rs
similarity index 100%
rename from datafusion/ffi/tests/cases/query_planner.rs
rename to datafusion/ffi/integration-tests/tests/cases/query_planner.rs
diff --git a/datafusion/ffi/tests/cases/udaf.rs b/datafusion/ffi/integration-tests/tests/cases/udaf.rs
similarity index 100%
rename from datafusion/ffi/tests/cases/udaf.rs
rename to datafusion/ffi/integration-tests/tests/cases/udaf.rs
diff --git a/datafusion/ffi/tests/cases/udf.rs b/datafusion/ffi/integration-tests/tests/cases/udf.rs
similarity index 100%
rename from datafusion/ffi/tests/cases/udf.rs
rename to datafusion/ffi/integration-tests/tests/cases/udf.rs
diff --git a/datafusion/ffi/tests/cases/udtf.rs b/datafusion/ffi/integration-tests/tests/cases/udtf.rs
similarity index 100%
rename from datafusion/ffi/tests/cases/udtf.rs
rename to datafusion/ffi/integration-tests/tests/cases/udtf.rs
diff --git a/datafusion/ffi/tests/cases/udwf.rs b/datafusion/ffi/integration-tests/tests/cases/udwf.rs
similarity index 100%
rename from datafusion/ffi/tests/cases/udwf.rs
rename to datafusion/ffi/integration-tests/tests/cases/udwf.rs
diff --git a/datafusion/ffi/tests/ffi_integration.rs b/datafusion/ffi/integration-tests/tests/ffi_integration.rs
similarity index 100%
rename from datafusion/ffi/tests/ffi_integration.rs
rename to datafusion/ffi/integration-tests/tests/ffi_integration.rs
diff --git a/datafusion/ffi/tests/utils/mod.rs b/datafusion/ffi/integration-tests/tests/utils/mod.rs
similarity index 100%
rename from datafusion/ffi/tests/utils/mod.rs
rename to datafusion/ffi/integration-tests/tests/utils/mod.rs
diff --git a/datafusion/ffi/src/tests/utils.rs b/datafusion/ffi/src/tests/utils.rs
index 2169cf0418..3eb09b80f6 100644
--- a/datafusion/ffi/src/tests/utils.rs
+++ b/datafusion/ffi/src/tests/utils.rs
@@ -36,7 +36,7 @@ fn find_cdylib(deps_dir: &Path) -> Result<PathBuf> {
"so"
};
- let pattern = format!("{lib_prefix}datafusion_ffi.{lib_ext}");
+ let pattern = format!("{lib_prefix}datafusion_ffi_integration_tests.{lib_ext}");
let lib_path = deps_dir.join(&pattern);
if lib_path.exists() {
diff --git a/xtask/src/ci_steps.rs b/xtask/src/ci_steps.rs
index 1f0a2406d6..bbb769ecee 100644
--- a/xtask/src/ci_steps.rs
+++ b/xtask/src/ci_steps.rs
@@ -282,6 +282,8 @@ impl StepContext {
"ci",
"-p",
"datafusion-ffi",
+ "-p",
+ "datafusion-ffi-integration-tests",
"--lib",
"--tests",
"--features",
@@ -790,6 +792,7 @@ cd /workspace && \
cargo test \
--profile ci \
-p datafusion-ffi \
+-p datafusion-ffi-integration-tests \
--lib \
--tests \
--features integration-tests
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org