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

refactor: implement new compare logic #32

Merged
merged 9 commits into from
Feb 3, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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
1 change: 0 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ jobs:
restore-keys: |
debug-${{ runner.os }}-${{ hashFiles('rust-toolchain') }}-
debug-${{ runner.os }}-
debug-${{ runner.os }}
- name: Install cargo-sort
run: |
cargo install cargo-sort
Expand Down
5 changes: 0 additions & 5 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,7 +1,2 @@
/target
# Ignore this output file, since the bad example will always fail.
# For normal case, output file should not be ignored, and it acts as
# a signal that our testcase need to be fixed.
examples/bad-case/simple/select.output
# Ignoring cargo.lock file
Cargo.lock
15 changes: 8 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ examples/
├── basic-case # Testcase root directory
│ └── simple # One environment
│ ├── config.toml # Config file for current environment
│ ├── select.result # Expected result file
│ ├── select.result # Output result file
│ └── select.sql # Input SQL testcase
├── basic.rs # Entrypoint of this example

Expand All @@ -32,14 +32,15 @@ cargo run --example basic
```
It will do following things:
1. Collect all environments(first-level directory) under `basic-case`.
2. Run testcases(`.sql` files) under environment one after one.
1. Write temporary result to `{testcase}.output`
2. Compare `{testcase}.output` with `{testcase}.result` using `diff`
2. Run tests(`.sql` files) under environment one after one.
1. Before execution it will read `{testcase}.result`(create one if not exists) to `Vec` as compare base.
jiacai2050 marked this conversation as resolved.
Show resolved Hide resolved
2. During execution it will collect query response and write to `{testcase}.result`
3. After execution it will diff content of `Vec` with `{testcase}.result`, **PASS** when they are the same otherwise **FAIL**.
jiacai2050 marked this conversation as resolved.
Show resolved Hide resolved
3. Report result.

Our target is to keep `*.result` file up to date, when `*.output` is equals to its corresponding result, the runner will delete it after executed.

When there is any diffs, the runner will keep `*.output` for later investigation.
Usually `result` files should be tracked in git, whenever there are failed tests, users should
1. Update `result` to latest version(e.g. `git add`) if result is right, or
jiacai2050 marked this conversation as resolved.
Show resolved Hide resolved
2. Restore `result` back to original version(e.g. `git checkout`), troubleshoot bugs in database implementation, and run tests again
jiacai2050 marked this conversation as resolved.
Show resolved Hide resolved

Below is the output of this example:
```bash
Expand Down
2 changes: 1 addition & 1 deletion examples/bad.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

//! A demo designed to run failed.
//!
//! When there is any diff between ${testcase}.output and ${testcase}.result,
//! When there is any diff between old and new result,
//! Users must resolve the diff, and keep the result file up to date.

use std::{fmt::Display, fs::File, path::Path};
Expand Down
16 changes: 4 additions & 12 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,10 @@ pub struct Config {
#[builder(default = "Config::default_test_case_extension()")]
#[serde(default = "Config::default_test_case_extension")]
pub test_case_extension: String,
/// Default value: `output`
#[builder(default = "Config::default_output_result_extension()")]
#[serde(default = "Config::default_output_result_extension")]
pub output_result_extension: String,
/// Default value: `result`
#[builder(default = "Config::default_expect_result_extension()")]
#[serde(default = "Config::default_expect_result_extension")]
pub expect_result_extension: String,
#[builder(default = "Config::default_result_extension()")]
#[serde(default = "Config::default_result_extension")]
pub result_extension: String,
/// Default value: `-- SQLNESS`
#[builder(default = "Config::default_interceptor_prefix()")]
#[serde(default = "Config::default_interceptor_prefix")]
Expand All @@ -44,11 +40,7 @@ impl Config {
"sql".to_string()
}

fn default_output_result_extension() -> String {
"output".to_string()
}

fn default_expect_result_extension() -> String {
fn default_result_extension() -> String {
"result".to_string()
}

Expand Down
2 changes: 1 addition & 1 deletion src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub enum SqlnessError {
#[error("IO operation failed, source error: {0}")]
IO(#[from] std::io::Error),

#[error("Cannot parse the output/result file. Not valid UTF-8 encoding")]
#[error("Cannot parse the result file. Not valid UTF-8 encoding")]
ReadResult(#[from] std::string::FromUtf8Error),

#[error("Run failed. {count} cases can't pass")]
Expand Down
3 changes: 2 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
//! │ ├── config.toml
//! │ ├── dml
//! │ │ └── basic.sql
//! │ │ └── basic.result
//! │ ├── ddl
//! │ └── system
//! └── remote
Expand All @@ -51,7 +52,7 @@
//! Note that only the first layer of sub-directory is special (for distinguash
//! different environments). All deeper layers are treated as the same. E.g.,
//! both `sqlness/local/dml/basic.sql` and `sqlness/local/dml/another-dir/basic.sql`
//! will be run under the `local` in the same pass.
//! will be run under the `local` env in the same pass.

mod case;
mod config;
Expand Down
110 changes: 49 additions & 61 deletions src/runner.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
// Copyright 2022 CeresDB Project Authors. Licensed under Apache-2.0.

use std::io::{Cursor, SeekFrom};
use std::path::{Path, PathBuf};
use std::str::FromStr;

use prettydiff::basic::{DiffOp, SliceChangeset};
use prettydiff::diff_lines;
use tokio::fs::{read_dir, remove_file, File, OpenOptions};
use tokio::io::{AsyncReadExt, AsyncWriteExt};
use tokio::fs::{read_dir, File, OpenOptions};
use tokio::io::{AsyncReadExt, AsyncSeekExt, AsyncWriteExt};
use tokio::time::Instant;
use walkdir::WalkDir;

Expand Down Expand Up @@ -124,15 +125,15 @@ impl<E: EnvController> Runner<E> {

async fn run_env(&self, env: &str, db: &E::DB) -> Result<()> {
let case_paths = self.collect_case_paths(env).await?;
let mut diff_cases = vec![];
let mut failed_cases = vec![];
let mut errors = vec![];
let start = Instant::now();
for path in case_paths {
let case_result = self.run_single_case(db, &path).await;
let is_success = self.run_single_case(db, &path).await;
let case_name = path.as_os_str().to_str().unwrap().to_owned();
match case_result {
Ok(true) => diff_cases.push(case_name),
Ok(false) => {}
match is_success {
Ok(false) => failed_cases.push(case_name),
Ok(true) => {}
Err(e) => {
if self.config.fail_fast {
println!("Case {case_name} failed with error {e:?}");
Expand All @@ -151,46 +152,66 @@ impl<E: EnvController> Runner<E> {
start.elapsed().as_millis()
);

let mut error_count = 0;
if !diff_cases.is_empty() {
println!("Different cases:");
println!("{diff_cases:#?}");
error_count += diff_cases.len();
if !failed_cases.is_empty() {
println!("Failed cases:");
println!("{failed_cases:#?}");
}

if !errors.is_empty() {
println!("Error cases:");
println!("{errors:#?}");
error_count += errors.len();
}

let error_count = failed_cases.len() + errors.len();
if error_count == 0 {
Ok(())
} else {
Err(SqlnessError::RunFailed { count: error_count })
}
}

async fn run_single_case(&self, db: &E::DB, path: &PathBuf) -> Result<bool> {
/// Return true when this case pass, otherwise false.
async fn run_single_case(&self, db: &E::DB, path: &Path) -> Result<bool> {
let case_path = path.with_extension(&self.config.test_case_extension);
let case = TestCase::from_file(case_path, &self.config).await?;
let output_path = path.with_extension(&self.config.output_result_extension);
let mut output_file = Self::open_output_file(&output_path).await?;
let case = TestCase::from_file(&case_path, &self.config).await?;
let result_path = path.with_extension(&self.config.result_extension);
let mut result_file = OpenOptions::new()
.create(true)
.write(true)
.read(true)
.open(&result_path)
.await?;

// Read old result out for compare later
let mut old_result = String::new();
result_file.read_to_string(&mut old_result).await?;

// Execute testcase
let mut new_result = Cursor::new(Vec::new());
let timer = Instant::now();
case.execute(db, &mut output_file).await?;
case.execute(db, &mut new_result).await?;
let elapsed = timer.elapsed();

output_file.flush().await?;
let is_different = self.compare(&path).await?;
if !is_different {
remove_file(output_path).await?;
// Truncate and write new result back
result_file.set_len(0).await?;
result_file.seek(SeekFrom::Start(0)).await?;
result_file.write_all(new_result.get_ref()).await?;

// Compare old and new result
let new_result = String::from_utf8(new_result.into_inner()).expect("not utf8 string");
if let Some(diff) = self.compare(&old_result, &new_result) {
println!("Result unexpected, path:{case_path:?}");
println!("{diff}");
return Ok(false);
}

println!(
"Test case {:?} finished, cost: {}ms",
path.as_os_str(),
elapsed.as_millis()
);
Ok(is_different)

Ok(true)
}

async fn collect_case_paths(&self, env: &str) -> Result<Vec<PathBuf>> {
Expand Down Expand Up @@ -229,48 +250,15 @@ impl<E: EnvController> Runner<E> {
Ok(cases)
}

async fn open_output_file<P: AsRef<Path>>(path: P) -> Result<File> {
Ok(OpenOptions::default()
.create(true)
.truncate(true)
.write(true)
.open(&path)
.await?)
}

/// Compare files' diff, return true if two files are different
async fn compare<P: AsRef<Path>>(&self, path: P) -> Result<bool> {
let mut result_lines = vec![];
File::open(
path.as_ref()
.with_extension(&self.config.expect_result_extension),
)
.await?
.read_to_end(&mut result_lines)
.await?;
let result_lines = String::from_utf8(result_lines)?;

let mut output_lines = vec![];
File::open(
path.as_ref()
.with_extension(&self.config.output_result_extension),
)
.await?
.read_to_end(&mut output_lines)
.await?;
let output_lines = String::from_utf8(output_lines)?;

let diff = diff_lines(&result_lines, &output_lines);
/// Compare result, return None if them are the same, else return diff changes
fn compare(&self, expected: &str, actual: &str) -> Option<String> {
let diff = diff_lines(expected, actual);
let diff = diff.diff();
let is_different = diff.iter().any(|d| !matches!(d, DiffOp::Equal(_)));
if is_different {
println!(
"Result unexpected, path:{}",
path.as_ref().to_string_lossy()
);
println!("{}", SliceChangeset { diff });
return Some(format!("{}", SliceChangeset { diff }));
}

Ok(is_different)
None
}
}