You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2026/09/25 17:43:51 UTC
[PR] docs: Make the preference for sqllogictest over Rust unit tests more visible [datafusion]
alamb opened a new pull request, #25756:
URL: https://github.com/apache/datafusion/pull/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: the rationale lives at the bottom of the `sqllogictests` section of the [testing guide](https://datafusion.apache.org/contributor-guide/testing.html#sqllogictests-tests), and the [Before Submitting a PR](https://datafusion.apache.org/contributor-guide/index.html#before-submitting-a-pr) section only says to *run* the tests, not to follow the testing guidelines. `AGENTS.md` does not mention it at all.
As a result, 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?
- `docs/source/contributor-guide/testing.md`: add a new "Choosing What Kind of Test to Write" section near the top that states the order of preference (`.slt`, then end-to-end API tests, then Rust unit tests) and the rationale (no recompile cycle, `--complete` auto-update, less code to maintain). The rationale that previously lived in the `sqllogictests` section is moved there, and that section now links to the new section and to the sqllogictest README cookbook.
- `docs/source/contributor-guide/index.md`: "Before Submitting a PR" now says to follow the testing guidelines (prefer `.slt` and end-to-end API tests where possible), not just run the quick start commands.
- `docs/source/contributor-guide/pr_review.md`: link the existing "prefer `.slt`" review guideline to the new section.
- `AGENTS.md`: add the guidance to the "Testing" section and link the new section from "Developer Documentation", so agents creating PRs pick it up.
## What is the testing strategy for this PR?
Documentation only. Ran `./ci/scripts/doc_prettier_check.sh --write --allow-dirty`.
## Are there any user-facing changes?
No, contributor documentation only.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs: Make the preference for sqllogictest over Rust unit tests more visible [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #25756:
URL: https://github.com/apache/datafusion/pull/25756
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs: Make the preference for sqllogictest over Rust unit tests more visible [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25756:
URL: https://github.com/apache/datafusion/pull/25756#issuecomment-5837371424
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25756?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 82.49%. Comparing base ([`e8e41ae`](https://app.codecov.io/gh/apache/datafusion/commit/e8e41ae958a5e6856ab9b0e7d8fb5deece0a9e91?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`79bb89f`](https://app.codecov.io/gh/apache/datafusion/commit/79bb89f18062b74a4320a7026d6921cb67f98563?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #25756 +/- ##
==========================================
- Coverage 82.49% 82.49% -0.01%
==========================================
Files 1140 1140
Lines 438529 438529
Branches 438529 438529
==========================================
- Hits 361783 361768 -15
- Misses 54874 54883 +9
- Partials 21872 21878 +6
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25756?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs: Make the preference for sqllogictest over Rust unit tests more visible [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #25756:
URL: https://github.com/apache/datafusion/pull/25756#discussion_r4107213329
##########
AGENTS.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
Review Comment:
try to prod agents along
##########
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
Review Comment:
we already documented that end user APIs were preferred, but it was somewhat buried
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs: Make the preference for sqllogictest over Rust unit tests more visible [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #25756:
URL: https://github.com/apache/datafusion/pull/25756#issuecomment-5857294079
Thanks @neilconway @asolimando and @2010YOUY01 -- let's try this and see if it helps future PRs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org