From f851611001b512e516cf1bd6fb49fd3f7273abfd Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Sun, 27 Mar 2016 14:26:05 -0700 Subject: [PATCH 1/7] tail: Pre-fill the buffer with zeroes Rather than fill the buffer on every file read iteration, pre-fill it with zeroes once at initialization time. --- src/tail/tail.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/tail/tail.rs b/src/tail/tail.rs index d89c95a60..996740e81 100755 --- a/src/tail/tail.rs +++ b/src/tail/tail.rs @@ -272,6 +272,8 @@ fn follow(mut reader: BufReader, settings: &Settings) { fn backwards_thru_file(file: &mut File, size: u64, buf: &mut Vec, should_stop: &mut F) where F: FnMut(u8) -> bool { + assert!(buf.len() >= BLOCK_SIZE as usize); + let max_blocks_to_read = (size as f64 / BLOCK_SIZE as f64).ceil() as usize; for block_idx in 0..max_blocks_to_read { @@ -281,13 +283,6 @@ fn backwards_thru_file(file: &mut File, size: u64, buf: &mut Vec, should_ BLOCK_SIZE }; - // Ensure that the buffer is filled and zeroed, if needed. - if buf.len() < (block_size as usize) { - for _ in buf.len()..(block_size as usize) { - buf.push(0); - } - } - // Seek backwards by the next block, read the full block into // `buf`, and then seek back to the start of the block again. let pos = file.seek(SeekFrom::Current(-(block_size as i64))).unwrap(); @@ -327,7 +322,7 @@ fn bounded_tail(mut file: File, settings: &Settings) { return; } - let mut buf = Vec::with_capacity(BLOCK_SIZE as usize); + let mut buf = vec![0; BLOCK_SIZE as usize]; // Find the position in the file to start printing from. match settings.mode { From 2b2c2b64c26487387b4f536f7ed78babc30add23 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Sun, 27 Mar 2016 14:29:47 -0700 Subject: [PATCH 2/7] tail: When tailing a file in bytes mode, seek directly to the specified byte When tailing a file, as opposed to stdin, and we are tailing bytes rather than lines, we can seek the requested number of bytes from the end of the file. This side steps the whole `backwards_thru_file` file loop and blocks of reads. Fixes #833. --- src/tail/tail.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/tail/tail.rs b/src/tail/tail.rs index 996740e81..865f4f40c 100755 --- a/src/tail/tail.rs +++ b/src/tail/tail.rs @@ -336,11 +336,8 @@ fn bounded_tail(mut file: File, settings: &Settings) { } }); }, - FilterMode::Bytes(mut count) => { - backwards_thru_file(&mut file, size, &mut buf, &mut |_| { - count -= 1; - count == 0 - }); + FilterMode::Bytes(count) => { + file.seek(SeekFrom::End(-(count as i64))).unwrap(); }, } From a629bb30767d27c85863afc518b1e0738f962413 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Sat, 26 Mar 2016 12:27:54 -0700 Subject: [PATCH 3/7] tests: Create the `ScopedFile` type for temporary files in tests This commit adds the `ScopedFile` type, which wraps and derefs to a `File`. When a `ScopedFile` is dropped, it removes the underlying file from the filesystem. This is useful for temporary, generated files in tests. --- tests/common/util.rs | 46 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) mode change 100644 => 100755 tests/common/util.rs diff --git a/tests/common/util.rs b/tests/common/util.rs old mode 100644 new mode 100755 index 8225f921a..686cee569 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -5,6 +5,7 @@ extern crate tempdir; use std::env; use std::fs::{self, File}; use std::io::{Read, Write, Result}; +use std::ops::{Deref, DerefMut}; #[cfg(unix)] use std::os::unix::fs::symlink as symlink_file; #[cfg(windows)] @@ -28,7 +29,7 @@ static ALREADY_RUN: &'static str = " you have already run this UCommand, if you another command in the same test, use TestSet::new instead of \ testing();"; static MULTIPLE_STDIN_MEANINGLESS: &'static str = "Ucommand is designed around a typical use case of: provide args and input stream -> spawn process -> block until completion -> return output streams. For verifying that a particular section of the input stream is what causes a particular behavior, use the Command type directly."; - + #[macro_export] macro_rules! assert_empty_stderr( ($cond:expr) => ( @@ -122,6 +123,40 @@ pub fn recursive_copy(src: &Path, dest: &Path) -> Result<()> { Ok(()) } +/// A scoped, temporary file that is removed upon drop. +pub struct ScopedFile { + path: PathBuf, + file: File, +} + +impl ScopedFile { + fn new(path: PathBuf, file: File) -> ScopedFile { + ScopedFile { + path: path, + file: file + } + } +} + +impl Deref for ScopedFile { + type Target = File; + fn deref(&self) -> &File { + &self.file + } +} + +impl DerefMut for ScopedFile { + fn deref_mut(&mut self) -> &mut File { + &mut self.file + } +} + +impl Drop for ScopedFile { + fn drop(&mut self) { + fs::remove_file(&self.path).unwrap(); + } +} + pub struct AtPath { pub subdir: PathBuf, } @@ -185,6 +220,9 @@ impl AtPath { Err(e) => panic!("{}", e), } } + pub fn make_scoped_file(&self, name: &str) -> ScopedFile { + ScopedFile::new(self.plus(name), self.make_file(name)) + } pub fn touch(&self, file: &str) { log_info("touch", self.plus_as_string(file)); File::create(&self.plus(file)).unwrap(); @@ -393,7 +431,7 @@ impl UCommand { .stderr(Stdio::piped()) .spawn() .unwrap(); - + result.stdin .take() .unwrap_or_else( @@ -401,8 +439,8 @@ impl UCommand { "Could not take child process stdin")) .write_all(&input) .unwrap_or_else(|e| panic!("{}", e)); - - result.wait_with_output().unwrap() + + result.wait_with_output().unwrap() } None => { self.raw.output().unwrap() From 1be7d31d5a7fdcdff1190f907130bf8e04e54c30 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Sat, 26 Mar 2016 12:36:15 -0700 Subject: [PATCH 4/7] tests/tail: Refactor the `test_single_big_args` test to use `ScopedFile` --- tests/tail.rs | 50 ++++++++++++++++++++------------------------------ 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/tests/tail.rs b/tests/tail.rs index 7d53bede1..5a0e3527d 100644 --- a/tests/tail.rs +++ b/tests/tail.rs @@ -9,10 +9,6 @@ static UTIL_NAME: &'static str = "tail"; static INPUT: &'static str = "foobar.txt"; -static BIG: &'static str = "big.txt"; - -static BIG_EXPECTED: &'static str = "big_single_big_args.expected"; - #[test] fn test_stdin_default() { let (at, mut ucmd) = testing(UTIL_NAME); @@ -27,35 +23,29 @@ fn test_single_default() { assert_eq!(result.stdout, at.read("foobar_single_default.expected")); } -const BIG_LINES: usize = 1_000_000; -const BIG_N_ARG: usize = 100_000; - -fn generate_big_test_files(at: &AtPath) { - let mut big_input = at.make_file(BIG); - for i in 0..BIG_LINES { - write!(&mut big_input, "Line {}\n", i).expect("Could not write to BIG file"); - } - big_input.flush().expect("Could not flush BIG file"); - - let mut big_expected = at.make_file(BIG_EXPECTED); - for i in (BIG_LINES - BIG_N_ARG)..BIG_LINES { - write!(&mut big_expected, "Line {}\n", i).expect("Could not write to BIG_EXPECTED file"); - } - big_expected.flush().expect("Could not flush BIG_EXPECTED file"); -} - -fn cleanup_big_test_files(at: &AtPath) { - at.cleanup(BIG); - at.cleanup(BIG_EXPECTED); -} - #[test] fn test_single_big_args() { + const FILE: &'static str = "single_big_args.txt"; + const EXPECTED_FILE: &'static str = "single_big_args_expected.txt"; + const LINES: usize = 1_000_000; + const N_ARG: usize = 100_000; + let (at, mut ucmd) = testing(UTIL_NAME); - generate_big_test_files(&at); - let result = ucmd.arg(BIG).arg("-n").arg(format!("{}", BIG_N_ARG)).run(); - assert_eq!(result.stdout, at.read(BIG_EXPECTED)); - cleanup_big_test_files(&at); + + let mut big_input = at.make_scoped_file(FILE); + for i in 0..LINES { + write!(&mut big_input, "Line {}\n", i).expect("Could not write to FILE"); + } + big_input.flush().expect("Could not flush FILE"); + + let mut big_expected = at.make_scoped_file(EXPECTED_FILE); + for i in (LINES - N_ARG)..LINES { + write!(&mut big_expected, "Line {}\n", i).expect("Could not write to EXPECTED_FILE"); + } + big_expected.flush().expect("Could not flush EXPECTED_FILE"); + + let result = ucmd.arg(FILE).arg("-n").arg(format!("{}", N_ARG)).run(); + assert_eq!(result.stdout, at.read(EXPECTED_FILE)); } #[test] From 0bc05e2dcf5e05e3d9bad4a173840ac883a0ecb5 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Sun, 27 Mar 2016 14:33:55 -0700 Subject: [PATCH 5/7] tests/tail: Add a test for tail'ing large files in bytes mode --- tests/tail.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/tail.rs b/tests/tail.rs index 5a0e3527d..85497170c 100644 --- a/tests/tail.rs +++ b/tests/tail.rs @@ -61,3 +61,35 @@ fn test_bytes_stdin() { let result = ucmd.arg("-c").arg("13").run_piped_stdin(at.read(INPUT)); assert_eq!(result.stdout, at.read("foobar_bytes_stdin.expected")); } + +#[test] +fn test_bytes_big() { + const FILE: &'static str = "test_bytes_big.txt"; + const EXPECTED_FILE: &'static str = "test_bytes_big_expected.txt"; + const BYTES: usize = 1_000_000; + const N_ARG: usize = 100_000; + + let (at, mut ucmd) = testing(UTIL_NAME); + + let mut big_input = at.make_scoped_file(FILE); + for i in 0..BYTES { + let digit = std::char::from_digit((i % 10) as u32, 10).unwrap(); + write!(&mut big_input, "{}", digit).expect("Could not write to FILE"); + } + big_input.flush().expect("Could not flush FILE"); + + let mut big_expected = at.make_scoped_file(EXPECTED_FILE); + for i in (BYTES - N_ARG)..BYTES { + let digit = std::char::from_digit((i % 10) as u32, 10).unwrap(); + write!(&mut big_expected, "{}", digit).expect("Could not write to EXPECTED_FILE"); + } + big_expected.flush().expect("Could not flush EXPECTED_FILE"); + + let result = ucmd.arg(FILE).arg("-c").arg(format!("{}", N_ARG)).run().stdout; + let expected = at.read(EXPECTED_FILE); + + assert_eq!(result.len(), expected.len()); + for (actual_char, expected_char) in result.chars().zip(expected.chars()) { + assert_eq!(actual_char, expected_char); + } +} From 9a5209a7a4728aa95fcd5d4e4b1e4f624efbca22 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Sun, 27 Mar 2016 14:41:56 -0700 Subject: [PATCH 6/7] tests/tail: Rename `INPUT` to `FOOBAR_TXT` as there are more than one inputs --- tests/tail.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/tail.rs b/tests/tail.rs index 85497170c..d8045284a 100644 --- a/tests/tail.rs +++ b/tests/tail.rs @@ -7,19 +7,19 @@ use common::util::*; static UTIL_NAME: &'static str = "tail"; -static INPUT: &'static str = "foobar.txt"; +static FOOBAR_TXT: &'static str = "foobar.txt"; #[test] fn test_stdin_default() { let (at, mut ucmd) = testing(UTIL_NAME); - let result = ucmd.run_piped_stdin(at.read(INPUT)); + let result = ucmd.run_piped_stdin(at.read(FOOBAR_TXT)); assert_eq!(result.stdout, at.read("foobar_stdin_default.expected")); } #[test] fn test_single_default() { let (at, mut ucmd) = testing(UTIL_NAME); - let result = ucmd.arg(INPUT).run(); + let result = ucmd.arg(FOOBAR_TXT).run(); assert_eq!(result.stdout, at.read("foobar_single_default.expected")); } @@ -51,14 +51,14 @@ fn test_single_big_args() { #[test] fn test_bytes_single() { let (at, mut ucmd) = testing(UTIL_NAME); - let result = ucmd.arg("-c").arg("10").arg(INPUT).run(); + let result = ucmd.arg("-c").arg("10").arg(FOOBAR_TXT).run(); assert_eq!(result.stdout, at.read("foobar_bytes_single.expected")); } #[test] fn test_bytes_stdin() { let (at, mut ucmd) = testing(UTIL_NAME); - let result = ucmd.arg("-c").arg("13").run_piped_stdin(at.read(INPUT)); + let result = ucmd.arg("-c").arg("13").run_piped_stdin(at.read(FOOBAR_TXT)); assert_eq!(result.stdout, at.read("foobar_bytes_stdin.expected")); } From 0d281cf88632d0ba11042160f776f80311931176 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Sun, 27 Mar 2016 14:42:45 -0700 Subject: [PATCH 7/7] tests/tail: Test when `-n` is larger than the number of lines in the file --- tests/tail.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/tail.rs b/tests/tail.rs index d8045284a..ab4abaccd 100644 --- a/tests/tail.rs +++ b/tests/tail.rs @@ -23,6 +23,13 @@ fn test_single_default() { assert_eq!(result.stdout, at.read("foobar_single_default.expected")); } +#[test] +fn test_n_greater_than_number_of_lines() { + let (at, mut ucmd) = testing(UTIL_NAME); + let result = ucmd.arg("-n").arg("99999999").arg(FOOBAR_TXT).run(); + assert_eq!(result.stdout, at.read(FOOBAR_TXT)); +} + #[test] fn test_single_big_args() { const FILE: &'static str = "single_big_args.txt";