From 4e7fb8a35974e8badff0aca681c986ddcd734712 Mon Sep 17 00:00:00 2001 From: Ciao Date: Sat, 14 Sep 2024 20:19:31 +0800 Subject: [PATCH] feat(fix): error handling (#3) * fix bug in error handling * CI runs on multiple OSs --- .github/workflows/ci.yaml | 12 +++++--- src/cmd.rs | 39 ++------------------------ src/input_processor.rs | 58 +++++++++++++++++++++++++++++++++++++-- src/main.rs | 3 +- tests/tc_tests.rs | 42 ++++++++++++++++++++++++++-- 5 files changed, 107 insertions(+), 47 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 1e31c45..545f48d 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -11,7 +11,11 @@ env: jobs: build: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest, windows-latest] steps: - uses: actions/checkout@v4 @@ -23,12 +27,12 @@ jobs: - name: Check formatting run: cargo fmt -- --check - - name: Build + - name: Build on ${{ matrix.os }} run: cargo build --verbose - - name: Run tests + - name: Run tests on ${{ matrix.os }} run: cargo test --verbose - - name: Run clippy + - name: Run clippy on ${{ matrix.os }} run: cargo clippy -- -D warnings diff --git a/src/cmd.rs b/src/cmd.rs index 321f802..43b40b1 100644 --- a/src/cmd.rs +++ b/src/cmd.rs @@ -1,8 +1,7 @@ use clap::{Parser, ValueEnum}; -use std::fs::File; -use std::io::{self, BufReader}; +use std::io; -use crate::input_processor::{print_counts, process_input, CountOptions, InputCounts}; +use crate::input_processor::{process_inputs, CountOptions}; #[derive(Parser)] #[command(author, version, about = "A simple count program by Rust and Cursor")] @@ -78,37 +77,5 @@ impl Cli { pub fn run() -> io::Result<()> { let (cli, options) = Cli::parse_args(); - - let mut stdout = io::stdout(); - let mut total_counts = InputCounts::default(); - let mut file_count = 0; - - if cli.files.is_empty() { - let stdin = io::stdin(); - let mut reader = BufReader::new(stdin.lock()); - if let Err(err) = process_input(&mut reader, &mut stdout, &options, None) { - eprintln!("Error processing stdin: {}", err); - } - } else { - for filename in &cli.files { - match File::open(filename) { - Ok(mut file) => { - match process_input(&mut file, &mut stdout, &options, Some(filename)) { - Ok(counts) => { - total_counts += counts; - file_count += 1; - } - Err(err) => eprintln!("Error processing file '{}': {}", filename, err), - } - } - Err(err) => eprintln!("Error opening file '{}': {}", filename, err), - } - } - } - - if file_count > 1 { - print_counts(&mut stdout, &total_counts, &options, Some("total"))?; - } - - Ok(()) + process_inputs(&cli.files, &mut io::stdout(), &options) } diff --git a/src/input_processor.rs b/src/input_processor.rs index 1b83770..b29f247 100644 --- a/src/input_processor.rs +++ b/src/input_processor.rs @@ -1,5 +1,6 @@ use crate::cmd::TokenizerModel; -use std::io::{self, Read, Write}; +use std::fs::File; +use std::io::{self, BufReader, Read, Write}; use tiktoken_rs::{cl100k_base, o200k_base, p50k_base, p50k_edit, r50k_base}; pub struct CountOptions { @@ -36,7 +37,58 @@ impl std::ops::AddAssign for InputCounts { } } -pub fn process_input( +pub fn process_inputs( + files: &Vec, + writer: &mut W, + options: &CountOptions, +) -> io::Result<()> +where + W: Write, +{ + let mut total_counts = InputCounts::default(); + let mut file_count = 0; + let mut error_count = 0; + + if files.is_empty() { + let stdin = io::stdin(); + let mut reader = BufReader::new(stdin.lock()); + if let Err(err) = process_input(&mut reader, writer, options, None) { + error_count += 1; + eprintln!("tc: Error processing stdin: {}", err); + } + } else { + for filename in files { + match File::open(filename) { + Ok(mut file) => match process_input(&mut file, writer, options, Some(filename)) { + Ok(counts) => { + total_counts += counts; + } + Err(err) => { + error_count += 1; + eprintln!("tc: Error processing file '{}': {}", filename, err) + } + }, + Err(err) => { + error_count += 1; + eprintln!("tc: Error opening file '{}': {}", filename, err) + } + } + file_count += 1; + } + } + + if file_count > 1 { + print_counts(writer, &total_counts, options, Some("total"))?; + } + + if error_count > 0 { + Err(io::Error::new(io::ErrorKind::Other, "")) + } else { + Ok(()) + } +} + +fn process_input( reader: &mut R, writer: &mut W, options: &CountOptions, @@ -102,7 +154,7 @@ fn count_input(buffer: &[u8], options: &CountOptions) -> InputCounts { } } -pub fn print_counts( +fn print_counts( writer: &mut W, counts: &InputCounts, options: &CountOptions, diff --git a/src/main.rs b/src/main.rs index ea842d6..407c921 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,8 +4,7 @@ mod cmd; mod input_processor; fn main() { - if let Err(err) = cmd::run() { - eprintln!("Error: {}", err); + if cmd::run().is_err() { process::exit(1); } } diff --git a/tests/tc_tests.rs b/tests/tc_tests.rs index 27b1ff0..6f6109c 100644 --- a/tests/tc_tests.rs +++ b/tests/tc_tests.rs @@ -75,8 +75,8 @@ fn test_non_existent_file() { let mut cmd = Command::cargo_bin("tc").unwrap(); cmd.arg("non_existent_file.txt") .assert() - .success() - .stderr(predicate::str::contains("Error opening file")); + .failure() + .stderr(predicate::str::contains("tc: Error opening file")); } #[test] @@ -107,3 +107,41 @@ fn test_different_tokenizer_model() { .success() .stdout(" 1 2 13 4\n"); } + +#[test] +fn test_error_code_without_termination() { + let dir = tempdir().unwrap(); + let existing_file1 = dir.path().join("existing1.txt"); + let existing_file2 = dir.path().join("existing2.txt"); + let non_existent_file = dir.path().join("non_existent.txt"); + + // Create existing files + let mut file1 = File::create(&existing_file1).unwrap(); + writeln!(file1, "This is an existing file.").unwrap(); + + let mut file2 = File::create(&existing_file2).unwrap(); + writeln!(file2, "This is another existing file.").unwrap(); + + let mut cmd = Command::cargo_bin("tc").unwrap(); + cmd.arg(&existing_file1) + .arg(&non_existent_file) + .arg(&existing_file2) + .assert() + .failure() + .code(1) // Expect an error code of 1 + .stdout(predicate::str::contains(" 1 5 26 7")) // Output for first existing file + .stderr(predicate::str::contains("tc: Error opening file")) // Error message for non-existent file + .stdout(predicate::str::contains(" 1 5 31 7")) // Output for second existing file + .stdout(predicate::str::contains( + " 2 10 57 14 total", + )); // Total count + + // Verify that the process didn't terminate early by checking if it processed both existing files + let output = cmd.output().unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!(stdout.contains("existing1.txt")); + assert!(stdout.contains("existing2.txt")); + assert!(stdout.contains("total")); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("non_existent.txt")); +}