-
Notifications
You must be signed in to change notification settings - Fork 810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Break parquet CI into its own workflow, standardize step names #2190
Changes from 3 commits
4525f75
a22e8c0
af860fd
6ed163a
90babb8
9d2e8e1
2b12b90
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,9 +40,27 @@ jobs: | |
uses: ./.github/actions/setup-builder | ||
with: | ||
rust-version: stable | ||
- name: Tests with default features | ||
- name: Test | ||
run: | | ||
cargo test -p arrow-flight | ||
- name: Tests with all features | ||
- name: Test --all-features | ||
run: | | ||
cargo test -p arrow-flight --all-features | ||
|
||
clippy: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed the |
||
name: Clippy | ||
runs-on: ubuntu-latest | ||
container: | ||
image: amd64/rust | ||
steps: | ||
- uses: actions/checkout@v2 | ||
- name: Setup Rust toolchain | ||
uses: ./.github/actions/setup-builder | ||
with: | ||
rust-version: stable | ||
- name: Setup Clippy | ||
run: | | ||
rustup component add clippy | ||
- name: Run clippy | ||
run: | | ||
cargo clippy -p arrow-flight --all-features -- -D warnings |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
# Licensed to the Apache Software Foundation (ASF) under one | ||
# or more contributor license agreements. See the NOTICE file | ||
# distributed with this work for additional information | ||
# regarding copyright ownership. The ASF licenses this file | ||
# to you under the Apache License, Version 2.0 (the | ||
# "License"); you may not use this file except in compliance | ||
# with the License. You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, | ||
# software distributed under the License is distributed on an | ||
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
# KIND, either express or implied. See the License for the | ||
# specific language governing permissions and limitations | ||
# under the License. | ||
|
||
--- | ||
# tests for parquet crate | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These tests were extracted from rust.yml |
||
name: "parquet" | ||
|
||
on: | ||
pull_request: | ||
|
||
jobs: | ||
# test the crate | ||
linux-test: | ||
name: Test | ||
runs-on: ubuntu-latest | ||
container: | ||
image: amd64/rust | ||
env: | ||
# Disable full debug symbol generation to speed up CI build and keep memory down | ||
# "1" means line tables only, which is useful for panic tracebacks. | ||
RUSTFLAGS: "-C debuginfo=1" | ||
steps: | ||
- uses: actions/checkout@v2 | ||
with: | ||
submodules: true | ||
- name: Setup Rust toolchain | ||
uses: ./.github/actions/setup-builder | ||
with: | ||
rust-version: stable | ||
- name: Test | ||
run: | | ||
cargo test -p parquet | ||
- name: Test --all-features | ||
run: | | ||
cargo test -p parquet --all-features | ||
|
||
|
||
# test compilaton | ||
linux-features: | ||
name: Check Compilation | ||
runs-on: ubuntu-latest | ||
container: | ||
image: amd64/rust | ||
env: | ||
# Disable full debug symbol generation to speed up CI build and keep memory down | ||
# "1" means line tables only, which is useful for panic tracebacks. | ||
RUSTFLAGS: "-C debuginfo=1" | ||
steps: | ||
- uses: actions/checkout@v2 | ||
with: | ||
submodules: true | ||
- name: Setup Rust toolchain | ||
uses: ./.github/actions/setup-builder | ||
with: | ||
rust-version: stable | ||
- name: Check compilation | ||
run: | | ||
cargo check -p parquet | ||
- name: Check compilation --no-default-features | ||
run: | | ||
cargo check -p parquet --no-default-features | ||
- name: Check compilation --no-default-features --features arrow | ||
run: | | ||
cargo check -p parquet --no-default-features --features arrow | ||
- name: Check compilation --no-default-features --all-features | ||
run: | | ||
cargo check -p parquet --all-features | ||
- name: Check compilation --all-targets | ||
run: | | ||
cargo check -p parquet --all-targets | ||
- name: Check compilation --no-default-features --all-targets | ||
run: | | ||
cargo check -p parquet --no-default-features --all-targets | ||
- name: Check compilation --no-default-features --features-arrow --all-targets | ||
run: | | ||
cargo check -p parquet --no-default-features --features arrow --all-targets | ||
|
||
clippy: | ||
name: Clippy | ||
runs-on: ubuntu-latest | ||
container: | ||
image: amd64/rust | ||
steps: | ||
- uses: actions/checkout@v2 | ||
- name: Setup Rust toolchain | ||
uses: ./.github/actions/setup-builder | ||
with: | ||
rust-version: stable | ||
- name: Setup Clippy | ||
run: | | ||
rustup component add clippy | ||
- name: Run clippy | ||
run: | | ||
# Only run clippy for the library at this time, | ||
# as there are clippy errors for other targets | ||
cargo clippy -p parquet --all-features --lib -- -D warnings | ||
#cargo clippy -p parquet --all-targets --all-features -- -D warnings | ||
alamb marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,19 +55,6 @@ jobs: | |
# run tests on all workspace members with default feature list | ||
cargo test | ||
- name: Re-run tests on parquet crate with all features | ||
run: | | ||
cargo test -p parquet --all-features | ||
- name: Test compilation of parquet library crate with different feature combinations | ||
run: | | ||
cargo check -p parquet | ||
cargo check -p parquet --no-default-features | ||
cargo check -p parquet --no-default-features --features arrow | ||
cargo check -p parquet --all-features | ||
- name: Test compilation of parquet targets with different feature combinations | ||
run: | | ||
cargo check -p parquet --all-targets | ||
cargo check -p parquet --no-default-features --all-targets | ||
cargo check -p parquet --no-default-features --features arrow --all-targets | ||
|
||
|
||
windows-and-macos: | ||
|
@@ -94,34 +81,6 @@ jobs: | |
export RUSTFLAGS="-C debuginfo=0" | ||
cargo test | ||
|
||
clippy: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Each of the individual crate workflows now run clippy so there is no reason to run it in the workspace as well |
||
name: Clippy | ||
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
arch: [ amd64 ] | ||
rust: [ stable ] | ||
container: | ||
image: ${{ matrix.arch }}/rust | ||
env: | ||
# Disable full debug symbol generation to speed up CI build and keep memory down | ||
# "1" means line tables only, which is useful for panic tracebacks. | ||
RUSTFLAGS: "-C debuginfo=1" | ||
steps: | ||
- uses: actions/checkout@v2 | ||
with: | ||
submodules: true | ||
- name: Setup Rust toolchain | ||
uses: ./.github/actions/setup-builder | ||
with: | ||
rust-version: ${{ matrix.rust }} | ||
- name: Setup Clippy | ||
run: | | ||
rustup component add clippy | ||
- name: Run clippy | ||
run: | | ||
cargo clippy --features test_common --features prettyprint --features=async --all-targets --workspace -- -D warnings | ||
|
||
check_benches: | ||
name: Check Benchmarks (but don't run them) | ||
runs-on: ubuntu-latest | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -182,6 +182,10 @@ impl RleEncoder { | |
self.bit_writer.bytes_written() | ||
} | ||
|
||
pub fn is_empty(&self) -> bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Turns out clippy complains about this in parquet crate, so I fixed it |
||
self.bit_writer.bytes_written() == 0 | ||
} | ||
|
||
#[inline] | ||
pub fn consume(mut self) -> Result<Vec<u8>> { | ||
self.flush()?; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By breaking each command into its own step it becomes easier to see which check failed on CI. The actual checks run are the same