Skip to content

CI: build and run sqllogictests binary directly in extended workflow #20282

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.

Sign up for GitHub

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

Closed
kosiew wants to merge 15 commits into from
Closed

CI: build and run sqllogictests binary directly in extended workflow #20282

Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
2fa9377
Enhance CI workflow and optimize release profile settings
kosiew Feb 10, 2026
1b4dbee
fix: Include extended.yml and Cargo.toml in workflow trigger paths
kosiew Feb 11, 2026
be84d47
ci: run sqllogictest binary from crate dir
kosiew Feb 11, 2026
304d28d
revert fix: Include extended.yml and Cargo.toml in workflow trigger p…
kosiew Feb 11, 2026
7b5b2a9
feat: update release profile settings in Cargo.toml
kosiew Feb 11, 2026
8d9bcb8
trigger CI extended tests
kosiew Feb 9, 2026
3e64ee2
Revert "trigger CI extended tests"
kosiew Feb 11, 2026
e0ae244
formatting
kosiew Feb 11, 2026
cd967ba
Merge branch 'main' into extended-tests-20052
kosiew Feb 11, 2026
ceb7d76
fix: Update sqllogictest binary build process and adjust working dire…
kosiew Feb 12, 2026
882251a
fix: Simplify sqllogictest binary retrieval in extended tests workflow
kosiew Feb 12, 2026
2b5a995
test with cargo test
kosiew Feb 13, 2026
04b3bf1
Revert "test with cargo test"
kosiew Feb 13, 2026
937fab9
trigger ci test
kosiew Feb 23, 2026
b830912
PR20282 before merge main
kosiew Feb 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions .github/workflows/extended.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,18 @@ jobs:
uses: ./.github/actions/setup-builder
with:
rust-version: stable
- name: Build sqllogictest binary
run: |
TEST_BIN=$(cargo test --features backtrace,parquet_encryption --profile release-nonlto --test sqllogictests --no-run --message-format=json | sed -n 's/.*"executable":"\([^"]*\)".*/\1/p' | head -n 1)
Copy link
Copy Markdown
Contributor

@alamb alamb Feb 11, 2026

Choose a reason for hiding this comment

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

Rather than cargo test another idea would be to use cargo build 🤔

Also, what is the reason to use --message-format=json?

Finally, what is the head command for? I didn't see any obvious reason in

https://github.com/apache/datafusion/actions/runs/21890300850/job/63194519053

Perhaps you could add a comment explaining what those commands are for

Copy link
Copy Markdown
Contributor Author

@kosiew kosiew Feb 12, 2026

Choose a reason for hiding this comment

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

Thanks, good suggestions. I agree cargo build is clearer here since this step only compiles the sqllogictests target and does not run it.

I used --message-format=json to capture the exact emitted test binary path because Cargo places test executables under target/.../deps with a hash suffix. The head -n 1 was a simple way to select the first extracted executable path from the stream.

I’ll simplify and document this more clearly in the workflow so the intent is explicit.

if [ -z "$TEST_BIN" ]; then
echo "Could not find sqllogictests test binary"
exit 1
fi
echo "TEST_BIN=$TEST_BIN" >> "$GITHUB_ENV"
- name: Run sqllogictest
run: |
cargo test --features backtrace,parquet_encryption --profile release-nonlto --test sqllogictests -- --include-sqlite
(
Copy link
Copy Markdown
Contributor

@alamb alamb Feb 11, 2026

Choose a reason for hiding this comment

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

Why the ( and )?
Also, why does it need to do cd when the current version doesn't?

Copy link
Copy Markdown
Contributor Author

@kosiew kosiew Feb 12, 2026

Choose a reason for hiding this comment

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

The cd is needed because sqllogictests resolves test data via relative paths (for example test_files/ and ../../datafusion-testing/data/ in datafusion/sqllogictest/bin/sqllogictests.rs), so running from repo root can point it at the wrong locations. I added it after an earlier version without cd errored with test_files/ not found.

The subshell (...) was only used to scope cd so it would not affect the subsequent cargo clean. It is not strictly necessary.

I’ll remove the subshell and switch to a cleaner step-level working-directory: datafusion/sqllogictest for the test execution step.

cd datafusion/sqllogictest
"$TEST_BIN" --include-sqlite
)
cargo clean