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/09/27 15:37:18 UTC

(datafusion) branch gh-readonly-queue/main/pr-25756-f029b944c8d2f8e70f0c213f87160c12ddad6a09 created (now 266b827f5e)

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-25756-f029b944c8d2f8e70f0c213f87160c12ddad6a09
in repository https://gitbox.apache.org/repos/asf/datafusion.git


      at 266b827f5e docs: Make the preference for sqllogictest over Rust unit tests more visible (#25756)

This branch includes the following new commits:

     new 266b827f5e docs: Make the preference for sqllogictest over Rust unit tests more visible (#25756)

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: docs: Make the preference for sqllogictest over Rust unit tests more visible (#25756)

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-25756-f029b944c8d2f8e70f0c213f87160c12ddad6a09
in repository https://gitbox.apache.org/repos/asf/datafusion.git

commit 266b827f5e603f7807342ee857729796a72773c4
Author: Andrew Lamb <an...@nerdnetworks.org>
AuthorDate: Sun Sep 27 15:36:54 2026 +0000

    docs: Make the preference for sqllogictest over Rust unit tests more visible (#25756)
    
    ## Which issue does this PR close?
    
    - Closes #.
    
    ## Rationale for this change
    
    The contributor guide already says that `sqllogictest` (`.slt`) tests
    and end-to-end API tests are preferred over Rust unit tests, but the
    guidance is buried:
    
    As a result, I am finding that both humans and coding agents regularly
    add large Rust unit tests where a few lines of `.slt` would do (see for
    example
    https://github.com/apache/datafusion/pull/25744#discussion_r4107006055).
    
    Making the guidance more visible should help keep code growth down and
    tests less coupled to implementation details.
    
    ## What changes are included in this PR?
    
    Make the docs more explicit
    
    <img width="1122" height="815" alt="Screenshot 2026-09-25 at 1 53 00 PM"
    src="https://github.com/user-attachments/assets/b4e86753-1f0c-4272-ac5b-3ae0a46d7560"
    />
    <img width="1056" height="693" alt="Screenshot 2026-09-25 at 1 53 23 PM"
    src="https://github.com/user-attachments/assets/d9794650-0a77-4085-8ada-0b0bafd832f4"
    />
    
    
    ## What is the testing strategy for this PR?
    
    Doc only. Ran build locally
    
    ## Are there any user-facing changes?
    
    No, contributor documentation only.
    
    ---------
    
    Co-authored-by: Claude Fable 5.1 <no...@anthropic.com>
---
 AGENTS.md                                  |  6 +++++-
 docs/source/contributor-guide/index.md     |  7 ++++++-
 docs/source/contributor-guide/pr_review.md |  3 ++-
 docs/source/contributor-guide/testing.md   | 24 ++++++++++++++++++++++++
 4 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index 8fdf314ed4..c35c66a3a1 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -4,6 +4,7 @@
 
 - [Quick Start Setup](docs/source/contributor-guide/development_environment.md#quick-start)
 - [Testing Quick Start](docs/source/contributor-guide/testing.md#testing-quick-start)
+- [Choosing What Kind of Test to Write](docs/source/contributor-guide/testing.md#choosing-what-kind-of-test-to-write)
 - [Before Submitting a PR](docs/source/contributor-guide/index.md#before-submitting-a-pr)
 - [Reviewing Pull Requests](docs/source/contributor-guide/pr_review.md)
 - [Contributor Guide](docs/source/contributor-guide/index.md)
@@ -38,7 +39,10 @@ When creating a PR, you MUST follow the [PR template](.github/pull_request_templ
 
 ## Testing
 
-If documentation files changed then run 
+When adding tests, you MUST follow
+[Choosing What Kind of Test to Write](docs/source/contributor-guide/testing.md#choosing-what-kind-of-test-to-write):
+
+If documentation files changed then run
 ```bash
 ./ci/scripts/doc_prettier_check.sh --write --allow-dirty
 ```
diff --git a/docs/source/contributor-guide/index.md b/docs/source/contributor-guide/index.md
index 6f1a0f1c19..d6a16680dd 100644
--- a/docs/source/contributor-guide/index.md
+++ b/docs/source/contributor-guide/index.md
@@ -122,7 +122,12 @@ before merge.
 # ./dev/rust_lint.sh --write --allow-dirty
 ```
 
-You should also run any relevant commands from the [testing quick start](testing.md#testing-quick-start).
+Please ensure your PR follows the [testing guide](testing.md). In particular:
+
+- Prefer end-to-end Public API tests such as `sqllogictest` (`.slt`) and DataFrame API,
+  over Rust unit tests where possible. See
+  [Choosing What Kind of Test to Write](testing.md#choosing-what-kind-of-test-to-write).
+- Run any relevant commands from the [testing quick start](testing.md#testing-quick-start).
 
 ## Conventional Commits & Labeling PRs
 
diff --git a/docs/source/contributor-guide/pr_review.md b/docs/source/contributor-guide/pr_review.md
index 1154ae0e96..6166a741aa 100644
--- a/docs/source/contributor-guide/pr_review.md
+++ b/docs/source/contributor-guide/pr_review.md
@@ -133,7 +133,8 @@ Guidelines for evaluating tests:
 
 1. Prefer `sqllogictest` (`.slt`) tests or DataFrame API tests where
    possible, as they exercise **user-visible behavior** and are less coupled
-   to internal implementation details than unit tests.
+   to internal implementation details than unit tests (see
+   [Choosing What Kind of Test to Write](testing.md#choosing-what-kind-of-test-to-write)).
 2. Verify tests cover edge cases and common failure scenarios, not just the
    common successful path. However, it is NOT necessary to test every possible
    error path, especially if it is difficult to trigger or unlikely to occur in
diff --git a/docs/source/contributor-guide/testing.md b/docs/source/contributor-guide/testing.md
index 4c097ae498..3e39ef2eb1 100644
--- a/docs/source/contributor-guide/testing.md
+++ b/docs/source/contributor-guide/testing.md
@@ -74,6 +74,30 @@ cargo nextest run
 [the book]: https://doc.rust-lang.org/book/
 [cargo-nextest]: https://nexte.st/
 
+## Choosing What Kind of Test to Write
+
+When adding tests, prefer tests that exercise **user-visible behavior** over
+tests of internal implementation details. Doing so means your tests are more
+likely to keep working (and keep catching regressions) while the code is
+refactored. In rough order of preference:
+
+1. Use [sqllogictest](#sqllogictests-tests) (`.slt`) tests for any behavior that can
+   be expressed in SQL.
+
+2. End-to-end tests that use public APIs such as the DataFrame API, for behavior
+   that cannot be expressed in SQL. For example, DataFrame tests live in
+   [`datafusion/core/tests/dataframe`] and run as part of the `core_integration`
+   test binary:
+
+   ```shell
+   cargo test -p datafusion --test core_integration -- dataframe
+   ```
+
+3. Rust unit tests for logic that cannot practically be reached through
+   SQL or DataFrame public APIs.
+
+[`datafusion/core/tests/dataframe`]: https://github.com/apache/datafusion/tree/main/datafusion/core/tests/dataframe
+
 ## Unit tests
 
 Tests for code in an individual module are defined in the same source file with a `test` module, following Rust convention.


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org