remove redundant CI benchmark check, cleanups #2212
Conversation
| # do not produce debug symbols to keep memory usage down | ||
| export RUSTFLAGS="-C debuginfo=0" | ||
| cargo test | ||
|
|
||
| check_benches: |
There was a problem hiding this comment.
Here is some evidence that this is already covered
When I broke an arrow benchmark:
echo "fff" >> arrow/benches/string_dictionary_builder.rsThen I ran the check that is part of arrow:
arrow-rs/.github/workflows/arrow.yml
Line 88 in bc493d9
cargo check -p arrow --all-targets
Checking arrow v19.0.0 (/Users/alamb/Software/arrow-rs2/arrow)
error: expected one of `!` or `::`, found `<eof>`
--> arrow/benches/string_dictionary_builder.rs:71:1
|
71 | fff
| ^^^ expected one of `!` or `::`
error: could not compile `arrow` due to previous error
warning: build failed, waiting for other jobs to finish...Similarly, when I broke parquet
echo "ggg" >> parquet/benches/arrow_reader.rs A check run by the parquet tests also finds it
cargo check -p parquet --all-features --all-targets
Compiling parquet v19.0.0 (/Users/alamb/Software/arrow-rs2/parquet)
error: expected one of `!` or `::`, found `<eof>`
--> parquet/benches/arrow_reader.rs:695:1
|
695 | ggg
| ^^^ expected one of `!` or `::`
Codecov Report
@@ Coverage Diff @@
## master #2212 +/- ##
==========================================
- Coverage 82.57% 82.56% -0.01%
==========================================
Files 239 239
Lines 62269 62269
==========================================
- Hits 51416 51415 -1
- Misses 10853 10854 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. |
| @@ -76,6 +76,17 @@ jobs: | |||
| uses: ./.github/actions/setup-builder | |||
| with: | |||
| rust-version: stable | |||
|
|
|||
| # Run different tests for the library on its own as well as | |||
There was a problem hiding this comment.
I tried to clarify the intent of the tests
This also makes it clear (to me) that there should be 8 combinations, and so I also added the missing combination (--all-targets --all-features)
| run: | | ||
| cargo check -p parquet --no-default-features --features arrow --all-targets | ||
| cargo check -p parquet --all-targets --all-features |
There was a problem hiding this comment.
This is a the missing combination
| @@ -91,12 +102,15 @@ jobs: | |||
| - name: Check compilation --all-targets | |||
| run: | | |||
| cargo check -p parquet --all-targets | |||
| - name: Check compilation --no-default-features --all-targets | |||
| - name: Check compilation --all-targets --no-default-features | |||
There was a problem hiding this comment.
I moved --all-targets to the first part the command line to make it easier to verify that all the combinations were checked
Co-authored-by: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com>
|
Benchmark runs are scheduled for baseline = 8139f7b and contender = 48cc6c3. 48cc6c3 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
re #2149
Rationale for this change
Clean up CI so we don't have to deal with jobs that are not adding value
What changes are included in this PR?
Are there any user-facing changes?
No