Skip to content
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

Issue-105, Brro Compressor E2E Tests and test workflow imrpovement #117

Merged
merged 6 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions .github/workflows/build_and_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ jobs:
- uses: Swatinem/rust-cache@v2
with:
# rust-cache already handles all the sane defaults for caching rust builds.
# However because we are running seperate debug/release builds in parallel,
# we also need to add the runner and cargo_flags to the key so that a seperate cache is used.
# Otherwise only the last build to finish would get saved to the cache.
# However, because we are running separate debug/release builds in parallel,
# we also need to add the runner and cargo_flags to the key so that a separate cache is used.
# Otherwise, only the last build to finish would get saved to the cache.
key: ${{ matrix.runner }} - ${{ matrix.cargo_flags }}
- name: Install external deps for the prom-remote-api crate
run: sudo apt-get install protobuf-compiler
Expand All @@ -48,8 +48,12 @@ jobs:
- name: Ensure that all crates compile and have no warnings under every possible combination of features
# some things to explicitly point out:
# * clippy also reports rustc warnings and errors
# * clippy --all-targets causes clippy to run against tests and examples which it doesnt do by default.
run: cargo hack --feature-powerset clippy --all-targets --locked ${{ matrix.cargo_flags }} -- -D warnings
# * clippy --all-targets causes clippy to run against tests and examples which it doesn't do by default.
Copy link
Contributor

@rukai rukai Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could achieve this more simply as:

run:
  # Display all clippy lint failures as warnings
  cargo hack --feature-powerset clippy --all-targets --locked ${{ matrix.cargo_flags }}
  # Fail CI on the first crate to fail clippy lints
  cargo hack --feature-powerset clippy --all-targets --locked ${{ matrix.cargo_flags }} -- -D warnings

Its the -D warnings that causes clippy to abort and not continue on to other crates.
But we also need the -D warnings to actually return non-zero exit code and fail CI.

Its also worth noting that clippy is easy to run locally, it is automatically installed already and you can just cargo clippy to see most of the failures locally. (I think it skips tests and benches) Since we dont use any features in fft-compressor yet: cargo clippy --all-targets will test everything including tests/benches

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! I like this approach much more. I added this one to the CI job.

run: |
# Display all clippy lint failures as warnings
cargo hack --feature-powerset clippy --all-targets --locked ${{ matrix.cargo_flags }}
# Fail CI on the first crate to fail clippy lints
cargo hack --feature-powerset clippy --all-targets --locked ${{ matrix.cargo_flags }} -- -D warnings
- name: Ensure that tests pass
run: |
cargo test --doc ${{ matrix.cargo_flags }} --all-features -- --show-output --nocapture
Expand Down
149 changes: 149 additions & 0 deletions brro-compressor/tests/e2e.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
use brro_compressor::utils::error::calculate_error;
use std::fs;
use std::path::{Path, PathBuf};
use wavbrro::wavbrro::WavBrro;

const TEST_FILE_NAME: &str = "go_gc_heap_goal_bytes.wbro";
const TEST_COMPRESSED_FILE_NAME: &str = "go_gc_heap_goal_bytes.bro";
const TEST_WBRO_PATH: &str = "tests/wbros/go_gc_heap_goal_bytes.wbro";

#[test]
fn test_compressor_idw_lossless() {
test_lossless_compression("idw")
}

#[test]
fn test_compressor_idw_lossy() {
test_lossy_compression("fft")
}

#[test]
fn test_compressor_polynomial_lossless() {
test_lossless_compression("polynomial")
}

#[test]
fn test_compressor_polynomial_lossy() {
test_lossy_compression("fft")
}

#[test]
fn test_compressor_noop() {
test_lossless_compression("noop")
}

#[test]
fn test_compressor_fft_lossy() {
test_lossy_compression("fft")
}

#[test]
fn test_compressor_auto_lossless() {
test_lossless_compression("auto")
}

#[test]
fn test_compressor_auto_lossy() {
test_lossy_compression("auto")
}

fn test_lossless_compression(compressor: &str) {
test_compression_decompression_flow(compressor, 0, compare_samples_lossless)
}

fn test_lossy_compression(compressor: &str) {
test_compression_decompression_flow(compressor, 5, compare_samples_with_allowed_error)
}

#[test]
fn test_compressor_constant() {
// tests/wbros/uptime.wbro constant data which can be compressed by constant compressor
let test_dir = tempfile::tempdir().unwrap().into_path();
fs::copy("tests/wbros/uptime.wbro", test_dir.join("uptime.wbro")).unwrap();

run_compressor(&[
"--compressor",
"noop",
test_dir.join("uptime.wbro").to_str().unwrap(),
]);

run_compressor(&["-u", test_dir.join("uptime.bro").to_str().unwrap()]);

compare_samples_lossless(
&PathBuf::from("tests/wbros/uptime.wbro"),
&test_dir.join("uptime.wbro"),
)
}

/// Runs compression and decompression test for a specified compressor.
/// max_error is an error level, compression speed is set as the lowest (0).
///
/// It compresses tests/wbros/go_gc_duration_count.wbro file, decompresses
/// got .bro file, and compares the original .wbro and the decompressed one.
fn test_compression_decompression_flow(
compressor: &str,
allowed_error: u8,
compare_fn: fn(original: &Path, processed: &Path),
) {
let test_dir = prepare_test_dir();

// Running data compression
run_compressor(&[
"--compressor",
compressor,
"--error",
allowed_error.to_string().as_str(),
test_dir.join(TEST_FILE_NAME).to_str().unwrap(),
]);

// Running data decompression
run_compressor(&[
"-u",
test_dir.join(TEST_COMPRESSED_FILE_NAME).to_str().unwrap(),
]);

compare_fn(
&PathBuf::from(TEST_WBRO_PATH),
&test_dir.join(TEST_FILE_NAME),
)
}

/// Prepares test directory and copies test wbro file there.
fn prepare_test_dir() -> PathBuf {
let test_dir = tempfile::tempdir().unwrap().into_path();
fs::copy(TEST_WBRO_PATH, test_dir.join(TEST_FILE_NAME)).unwrap();
test_dir
}

/// Runs compressor binary with provided arguments.
fn run_compressor(args: &[&str]) {
let compressor_bin = env!("CARGO_BIN_EXE_brro-compressor");
let exit_status = std::process::Command::new(compressor_bin)
.args(args)
.status()
.unwrap();

assert!(exit_status.success());
}

fn compare_samples_lossless(original: &Path, uncompressed: &Path) {
let original_samples = WavBrro::from_file(original).unwrap();
let uncompressed_samples = WavBrro::from_file(uncompressed).unwrap();
assert_eq!(original_samples, uncompressed_samples);
}

fn compare_samples_with_allowed_error(original: &Path, uncompressed: &Path) {
const MAX_ALLOWED_ERROR: f64 = 0.05;

let original_samples = WavBrro::from_file(original).unwrap();
let uncompressed_samples = WavBrro::from_file(uncompressed).unwrap();
let err = calculate_error(&original_samples, &uncompressed_samples);

assert!(
err <= MAX_ALLOWED_ERROR,
"Error: {}\nOriginal : {:?}\nUncompressed: {:?}",
err,
original_samples,
uncompressed_samples
);
}
Loading