alamb commented on code in PR #25203:
URL: https://github.com/apache/datafusion/pull/25203#discussion_r3993447458


##########
.github/workflows/extended.yml:
##########
@@ -60,13 +47,14 @@ jobs:
   # Run extended tests (with feature 'extended_tests')
   linux-test-extended:
     name: cargo test 'extended_tests' (amd64)
+    if: github.event_name != 'pull_request'

Review Comment:
   I think it would help me to have a note here about what this is doing
   
   Something like
   ```yaml
   # long running job: only run on merges to main and merge_queue
   ```



##########
docs/source/contributor-guide/testing.md:
##########
@@ -127,13 +127,16 @@ cargo insta review
 
 ## Extended Tests
 
-In addition to the standard CI test suite that is run on all PRs prior to 
merge,
-DataFusion has "extended" tests (defined in [extended.yml]) that are run on 
each
-commit to `main`. These tests rarely fail but take significantly longer to run
-than the standard test suite and add important test coverage such as ensuring
-correctness when there are hash collisions and running the relevant portions of
-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`,

Review Comment:
   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



##########
docs/source/contributor-guide/testing.md:
##########
@@ -127,13 +127,16 @@ cargo insta review
 
 ## Extended Tests
 
-In addition to the standard CI test suite that is run on all PRs prior to 
merge,
-DataFusion has "extended" tests (defined in [extended.yml]) that are run on 
each
-commit to `main`. These tests rarely fail but take significantly longer to run
-than the standard test suite and add important test coverage such as ensuring
-correctness when there are hash collisions and running the relevant portions of
-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`,
+forced hash-collision tests, and the [sqlite test suite].
+
+On ordinary PR updates, GitHub reports these jobs as skipped to conserve CI

Review Comment:
   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



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to