Skip to content

Commit

Permalink
Harden size-based rotation test (use a constant OffsetDateTime)
Browse files Browse the repository at this point in the history
  • Loading branch information
CBenoit committed Feb 13, 2024
1 parent 08eae52 commit 617c289
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 24 deletions.
72 changes: 48 additions & 24 deletions tracing-appender/src/rolling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@ use std::{
fs::{self, File, OpenOptions},
io::{self, Write},
path::{Path, PathBuf},
sync::{
atomic::{AtomicU64, AtomicUsize, Ordering},
Arc,
},
sync::atomic::{AtomicU64, AtomicUsize, Ordering},
};
use time::{format_description, Date, Duration, OffsetDateTime, Time};

Expand Down Expand Up @@ -115,7 +112,7 @@ struct Inner {
date_format: Vec<format_description::FormatItem<'static>>,
rotation: Rotation,
next_date: AtomicUsize,
current_size: Arc<AtomicU64>,
current_size: AtomicU64,
max_files: Option<usize>,
}

Expand Down Expand Up @@ -194,27 +191,53 @@ impl RollingFileAppender {
}

fn from_builder(builder: &Builder, directory: impl AsRef<Path>) -> Result<Self, InitError> {
Self::impl_from_builder(
builder,
directory,
#[cfg(test)]
Box::new(OffsetDateTime::now_utc),
)
}

#[cfg(test)]
fn from_builder_with_custom_now(
builder: &Builder,
directory: impl AsRef<Path>,
now: Box<dyn Fn() -> OffsetDateTime + Send + Sync>,
) -> Result<Self, InitError> {
Self::impl_from_builder(builder, directory, now)
}

fn impl_from_builder(
builder: &Builder,
directory: impl AsRef<Path>,
#[cfg(test)] now: Box<dyn Fn() -> OffsetDateTime + Send + Sync>,
) -> Result<Self, InitError> {
let Builder {
ref rotation,
ref prefix,
ref suffix,
ref max_files,
} = builder;
rotation,
prefix,
suffix,
max_files,
} = &builder;

let directory = directory.as_ref().to_path_buf();
let now = OffsetDateTime::now_utc();
let (state, writer) = Inner::new(
now,
#[cfg(not(test))]
OffsetDateTime::now_utc(),
#[cfg(test)]
now(),
rotation.clone(),
directory,
prefix.clone(),
suffix.clone(),
*max_files,
)?;

Ok(Self {
state,
writer,
#[cfg(test)]
now: Box::new(OffsetDateTime::now_utc),
now,
})
}

Expand Down Expand Up @@ -645,7 +668,7 @@ impl Inner {
),
rotation,
max_files,
current_size: Arc::new(AtomicU64::new(0)),
current_size: AtomicU64::new(0),
};

if let Some(max_bytes) = inner.rotation.max_bytes {
Expand All @@ -660,7 +683,7 @@ impl Inner {
.metadata()
.map_err(builder::InitError::ctx("Failed to read file metadata"))?
.len();
inner.current_size = Arc::new(AtomicU64::new(current_file_size));
inner.current_size = AtomicU64::new(current_file_size);

let writer = RwLock::new(file);

Expand Down Expand Up @@ -996,7 +1019,6 @@ mod test {
use super::*;
use std::fs;
use std::io::Write;
use std::ops::Deref;

fn find_str_in_log(dir_path: &Path, expected_value: &str) -> bool {
let dir_contents = fs::read_dir(dir_path).expect("Failed to read directory");
Expand Down Expand Up @@ -1519,12 +1541,14 @@ mod test {

#[test]
fn size_based_rotation() {
let format = format_description::parse("size_based_rotation.[year]-[month]-[day]").unwrap();
let now = OffsetDateTime::now_utc();

let directory = tempfile::tempdir().expect("failed to create tempdir");

let mut appender = max_size(&directory, "size_based_rotation", 8);
let mut appender = RollingFileAppender::builder()
.rotation(Rotation::max_bytes(8))
.filename_prefix("size_based_rotation")
.filename_suffix("log")
.build_with_now(&directory, Box::new(|| OffsetDateTime::UNIX_EPOCH))
.expect("initializing rolling file appender failed");

writeln!(appender, "(file1) more than 8 bytes").unwrap();
writeln!(appender, "(file2) more than 8 bytes again").unwrap();
Expand All @@ -1534,12 +1558,12 @@ mod test {
println!("dir={:?}", dir_contents);

let expected_files = [
format!("{}.0", now.format(&format).unwrap()),
format!("{}.1", now.format(&format).unwrap()),
format!("{}.2", now.format(&format).unwrap()),
"size_based_rotation.1970-01-01.0.log",
"size_based_rotation.1970-01-01.1.log",
"size_based_rotation.1970-01-01.2.log",
];
let mut expected_files: std::collections::HashSet<&str> =
expected_files.iter().map(|s| s.deref()).collect();
expected_files.iter().copied().collect();

for entry in dir_contents {
println!("entry={:?}", entry);
Expand Down
9 changes: 9 additions & 0 deletions tracing-appender/src/rolling/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,15 @@ impl Builder {
pub fn build(&self, directory: impl AsRef<Path>) -> Result<RollingFileAppender, InitError> {
RollingFileAppender::from_builder(self, directory)
}

#[cfg(test)]
pub(super) fn build_with_now(
&self,
directory: impl AsRef<Path>,
now: Box<dyn Fn() -> time::OffsetDateTime + Send + Sync>,
) -> Result<RollingFileAppender, InitError> {
RollingFileAppender::from_builder_with_custom_now(self, directory, now)
}
}

impl Default for Builder {
Expand Down

0 comments on commit 617c289

Please sign in to comment.