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/06/15 18:20:52 UTC
[GitHub] [arrow-datafusion] alamb opened a new pull request, #6682: Minor: combine dataframe tests
alamb opened a new pull request, #6682:
URL: https://github.com/apache/arrow-datafusion/pull/6682
# Which issue does this PR close?
Part of https://github.com/apache/arrow-datafusion/issues/6676
# Rationale for this change
We are running into builder space problems so reducing the space needed to run `cargo test` is required. Also it would be nice to get faster CI runs.
Locally for me on my mac, it seems like each test binary takes a few seconds to compile and 100+MB of disk space. Thus combining them into fewer larger tests (so we have fewer binaries) will be an improvement
# What changes are included in this PR?
Combine `dataframe` and `dataframe_function` tests:
# Are these changes tested?
Yes by existing CI
# Are there any user-facing changes?
No -- this is development process
# Measurements
## dataframe_functions (126MB)
```shell
cd /Users/alamb/Software/arrow-datafusion2/ && RUST_BACKTRACE=1 CARGO_TARGET_DIR=/Users/alamb/Software/target-df2 cargo test --test dataframe_functions
Finished test [unoptimized + debuginfo] target(s) in 0.28s
Running tests/dataframe_functions.rs (/Users/alamb/Software/target-df2/debug/deps/dataframe_functions-6259330cccb05fd4)
...
(arrow_dev) alamb@MacBook-Pro-8:~/Software/arrow-datafusion2$ du -s -h /Users/alamb/Software/target-df2/debug/deps/dataframe_functions-6259330cccb05fd4
126M /Users/alamb/Software/target-df2/debug/deps/dataframe_functions-6259330cccb05fd4
```
## dataframe (pre-PR) (133MB)
```rust
cd /Users/alamb/Software/arrow-datafusion2/ && RUST_BACKTRACE=1 CARGO_TARGET_DIR=/Users/alamb/Software/target-df2 cargo test --test dataframe
Compiling datafusion v26.0.0 (/Users/alamb/Software/arrow-datafusion2/datafusion/core)
Finished test [unoptimized + debuginfo] target(s) in 4.65s
Running tests/dataframe.rs (/Users/alamb/Software/target-df2/debug/deps/dataframe-bb6b871995f5ee03)
(arrow_dev) alamb@MacBook-Pro-8:~/Software/arrow-datafusion2$ du -s -h /Users/alamb/Software/target-df2/debug/deps/dataframe-bb6b871995f5ee03
133M /Users/alamb/Software/target-df2/debug/deps/dataframe-bb6b871995f5ee03
```
## dataframe (with this PR) (134MB)
By combining them, the result is 134 MB, A savings of 125MB.
```rust
cd /Users/alamb/Software/arrow-datafusion2/ && RUST_BACKTRACE=1 CARGO_TARGET_DIR=/Users/alamb/Software/target-df2 cargo test --test dataframe
Compiling datafusion v26.0.0 (/Users/alamb/Software/arrow-datafusion2/datafusion/core)
Finished test [unoptimized + debuginfo] target(s) in 4.98s
Running tests/dataframe.rs (/Users/alamb/Software/target-df2/debug/deps/dataframe-bb6b871995f5ee03)
...
du -s -h /Users/alamb/Software/target-df2/debug/deps/dataframe-bb6b871995f5ee03
134M /Users/alamb/Software/target-df2/debug/deps/dataframe-bb6b871995f5ee03
```
Admittedly these measurements are on my mac, but I think they will hold generally
--
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 commented on a diff in pull request #6682: Minor: combine dataframe tests to save build time and builder disk space
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #6682:
URL: https://github.com/apache/arrow-datafusion/pull/6682#discussion_r1231396607
##########
datafusion/core/tests/dataframe.rs:
##########
@@ -1267,3 +1266,676 @@ async fn use_var_provider() -> Result<()> {
dataframe.collect().await?;
Ok(())
}
+
+// --------------- Begin dataframe function tests ----------------
Review Comment:
these tests are moved from one file to another and `create_test_table` redundancy is removed. Otherwise they are not altered
--
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 commented on pull request #6682: Minor: combine `dataframe` and `dataframe_functions` tests to save build time and builder disk space
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6682:
URL: https://github.com/apache/arrow-datafusion/pull/6682#issuecomment-1593567314
I verified that the moved tests still run. For example:
https://github.com/apache/arrow-datafusion/actions/runs/5282277762/jobs/9557123708?pr=6682
--
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 commented on a diff in pull request #6682: Minor: combine `dataframe` and `dataframe_functions` tests to save build time and builder disk space
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #6682:
URL: https://github.com/apache/arrow-datafusion/pull/6682#discussion_r1232110179
##########
datafusion/core/tests/dataframe.rs:
##########
@@ -1267,3 +1266,676 @@ async fn use_var_provider() -> Result<()> {
dataframe.collect().await?;
Ok(())
}
+
+// --------------- Begin dataframe function tests ----------------
Review Comment:
This is a good idea -- I implemented a modified version of this suggestion in https://github.com/apache/arrow-datafusion/pull/6697
--
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] crepererum commented on a diff in pull request #6682: Minor: combine `dataframe` and `dataframe_functions` tests to save build time and builder disk space
Posted by "crepererum (via GitHub)" <gi...@apache.org>.
crepererum commented on code in PR #6682:
URL: https://github.com/apache/arrow-datafusion/pull/6682#discussion_r1232024517
##########
datafusion/core/tests/dataframe.rs:
##########
@@ -1267,3 +1266,676 @@ async fn use_var_provider() -> Result<()> {
dataframe.collect().await?;
Ok(())
}
+
+// --------------- Begin dataframe function tests ----------------
Review Comment:
I suggest you put these into rust modules instead. The issue w/ disk space is NOT that they are in different files but that you have two test binaries.
File struct is:
- `dataframe.rs`:
```rust
mod dataframe;
mod dataframe_functions;
```
- `dataframe/dataframe.rs`
- `dataframe/dataframe_functions.rs`
--
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 closed pull request #6682: Minor: combine `dataframe` and `dataframe_functions` tests to save build time and builder disk space
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed pull request #6682: Minor: combine `dataframe` and `dataframe_functions` tests to save build time and builder disk space
URL: https://github.com/apache/arrow-datafusion/pull/6682
--
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