Skip to content

ci: require extended tests in the merge queue - #25203

Merged
kumarUjjawal merged 3 commits into
apache:mainfrom
kumarUjjawal:ci/21241-extended-tests-merge-queue
Sep 12, 2026
Merged

ci: require extended tests in the merge queue#25203
kumarUjjawal merged 3 commits into
apache:mainfrom
kumarUjjawal:ci/21241-extended-tests-merge-queue

Conversation

@kumarUjjawal

@kumarUjjawal kumarUjjawal commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Extended-test failures can reach main because the checks are optional on PRs and run after merge. Require all three suites in the merge queue so failures block merging without running the suites on every PR update.

What changes are included in this PR?

  • Add an unfiltered pull_request trigger and a merge_group trigger. Skip the three jobs on PR updates so their required checks allow queue entry.
  • Add the Rust extended tests, forced hash-collision tests, and SQLite SQL logic tests to the required checks in .asf.yaml.
  • Remove duplicate runs on pushes to main. Preserve release-branch pushes and manual dispatch, including the optional PR head SHA.
  • Update the contributor testing guide.

Test commands, feature flags, runners, concurrency settings, and benchmark checks are unchanged.

What is the testing strategy for this PR?

This change only affects CI scheduling and documentation, so it adds no Rust tests.

  • The required-status-check validator passes for all 35 configured checks.
  • YAML comparison confirms that the three jobs are unchanged except for their conditions. PR and merge-group triggers have no path filters.
  • Documentation formatting, workflow tool-install checks, and git diff --check pass.
  • Actionlint 1.7.12 reports the same unsupported parallel blocks on the base and this branch. Both workflows pass when those existing blocks are flattened in temporary lint inputs.

The initial hosted PR run reports all three extended jobs as skipped. The hosted required-status-check validator also passes. Merge-group execution and enforcement still need validation when the PR enters the queue.

Are there any user-facing changes?

Contributors wait for extended tests in the merge queue instead of on PR updates or after merge. There are no query behavior or public API changes.

@github-actions github-actions Bot added documentation Improvements or additions to documentation development-process Related to development process of DataFusion labels Sep 11, 2026
@kumarUjjawal

Copy link
Copy Markdown
Contributor Author

cc @alamb @comphead @Jefffrey

@kumarUjjawal

Copy link
Copy Markdown
Contributor Author

Another think I was thinking was running only the required ci jobs on certain prs. For example a PR changing only docs should not run the full suite, and we can skip all the rust jobs on such prs.

@codecov-commenter

codecov-commenter commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.93%. Comparing base (bb21f51) to head (8c5a8d6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25203      +/-   ##
==========================================
- Coverage   81.93%   81.93%   -0.01%     
==========================================
  Files        1133     1133              
  Lines      423529   423529              
  Branches   423529   423529              
==========================================
- Hits       347032   347024       -8     
- Misses      55907    55914       +7     
- Partials    20590    20591       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@alamb

alamb commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Another think I was thinking was running only the required ci jobs on certain prs. For example a PR changing only docs should not run the full suite, and we can skip all the rust jobs on such prs.

Yes, this is a good idea. I think it got disabled in the past because there is some doc pages, like the config settings, that actually do need to run the check_config_settings job:

We originally had it in

I think the issue is something related to not being able to trigger for merge groups

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @kumarUjjawal -- the code makes sense to me (and Claude code)

# Run extended tests (with feature 'extended_tests')
linux-test-extended:
name: cargo test 'extended_tests' (amd64)
if: github.event_name != 'pull_request'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it would help me to have a note here about what this is doing

Something like

# long running job: only run on merges to main and merge_queue

merging into `main`. All three jobs must pass: Rust tests with `extended_tests`,
forced hash-collision tests, and the [sqlite test suite].

On ordinary PR updates, GitHub reports these jobs as skipped to conserve CI

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure we need to talk about ". These skipped checks allow the PR to enter the merge queue, where the jobs run against the combined merge-group commit." -- that is perhaps a somewhat irrelevant implementation detail for contributors

the entire [sqlite test suite]. You can run the extended tests
locally by following the [instructions in the documentation].
DataFusion runs the extended tests in [extended.yml] in the merge queue before
merging into `main`. All three jobs must pass: Rust tests with `extended_tests`,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we need to enumerate the test names here

I do think it would be nice to keep the rationale that the tests take a long time to run but do often fail

@kumarUjjawal

Copy link
Copy Markdown
Contributor Author

Thank you @alamb

@kumarUjjawal
kumarUjjawal added this pull request to the merge queue Sep 12, 2026
Merged via the queue into apache:main with commit 615e9b4 Sep 12, 2026
42 checks passed
@kumarUjjawal
kumarUjjawal deleted the ci/21241-extended-tests-merge-queue branch September 12, 2026 11:42
@alamb

alamb commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Thank you!

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

Labels

development-process Related to development process of DataFusion documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move Extended tests to the standard ones?

3 participants