You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/05/15 20:50:44 UTC

[GitHub] [arrow-datafusion] alamb opened a new pull request, #6357: Minor: Update the testing section of contributor guide

alamb opened a new pull request, #6357:
URL: https://github.com/apache/arrow-datafusion/pull/6357

   # Which issue does this PR close?
   
   N/A 
   
   # Rationale for this change
   
   @mustafasrepo  and I had a good conversation on https://github.com/apache/arrow-datafusion/pull/6234 (https://github.com/apache/arrow-datafusion/pull/6234#issuecomment-1542157223) about the rationale for sqllogictests and I wanted to include a summary of that discussion in the docs
   
   # What changes are included in this PR?
   
   Update the contribution guide to be clearer about tests and the *why* of DataFusion using sqllogictests
   
   # Are these changes tested?
   N/A
   
   # Are there any user-facing changes?
   
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!--
   If there are any breaking changes to public APIs, please add the `api change` label.
   -->


-- 
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@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] viirya commented on a diff in pull request #6357: Minor: Update the testing section of contributor guide

Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #6357:
URL: https://github.com/apache/arrow-datafusion/pull/6357#discussion_r1194391648


##########
docs/source/contributor-guide/index.md:
##########
@@ -115,42 +115,41 @@ or run them all at once:
 
 - [dev/rust_lint.sh](../../../dev/rust_lint.sh)
 
-### Test Organization
+## Testing
 
-Tests are very important to ensure that improvemens or fixes are not accidentally broken during subsequent refactorings.
+Tests are critical to ensure that DataFusion is working properly and
+is not accidentally broken during refactorings. All new features
+should have test coverage.
 
 DataFusion has several levels of tests in its [Test
 Pyramid](https://martinfowler.com/articles/practical-test-pyramid.html)
-and tries to follow rust standard [Testing Organization](https://doc.rust-lang.org/book/ch11-03-test-organization.html) in the The Book.
+and tries to follow the Rust standard [Testing Organization](https://doc.rust-lang.org/book/ch11-03-test-organization.html) in the The Book.
 
-This section highlights the most important test modules that exist
+### Unit tests
 
-#### Unit tests
+Tests for code in an individual module are defined in the same source file with a `test` module, following Rust convention.
 

Review Comment:
   👍 Looks more clear.



-- 
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@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb merged pull request #6357: Minor: Update the testing section of contributor guide

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #6357:
URL: https://github.com/apache/arrow-datafusion/pull/6357


-- 
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@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org