Skip to content

Commit e0363fc

Browse files
tmccombstmccombs
committed
refactor: Avoid passing in current time just for tests
Instead use a thread local variable that we can use to set the current time, and use a separate now function in tests than the actual binary.
1 parent 9859d51 commit e0363fc

2 files changed

Lines changed: 75 additions & 74 deletions

File tree

‎src/filter/time.rs‎

Lines changed: 73 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,26 @@ pub enum TimeFilter {
99
After(SystemTime),
1010
}
1111

12+
#[cfg(not(test))]
13+
fn now() -> Zoned {
14+
Zoned::now()
15+
}
16+
17+
#[cfg(test)]
18+
thread_local! {
19+
static TESTTIME: std::cell::RefCell<Option<Zoned>> = None.into();
20+
}
21+
22+
/// This allows us to set a specific time when running tests
23+
#[cfg(test)]
24+
fn now() -> Zoned {
25+
TESTTIME.with_borrow(|reftime| reftime.as_ref().cloned().unwrap_or_else(|| Zoned::now()))
26+
}
27+
1228
impl TimeFilter {
13-
fn from_str(ref_time: &SystemTime, s: &str) -> Option<SystemTime> {
29+
fn from_str(s: &str) -> Option<SystemTime> {
1430
s.parse::<Span>()
15-
.and_then(|duration| {
16-
Zoned::try_from(*ref_time).and_then(|zdt| zdt.checked_sub(duration))
17-
})
31+
.and_then(|duration| now().checked_sub(duration))
1832
.ok()
1933
.or_else(|| {
2034
let local_tz = TimeZone::system();
@@ -37,12 +51,12 @@ impl TimeFilter {
3751
.map(SystemTime::from)
3852
}
3953

40-
pub fn before(ref_time: &SystemTime, s: &str) -> Option<TimeFilter> {
41-
TimeFilter::from_str(ref_time, s).map(TimeFilter::Before)
54+
pub fn before(s: &str) -> Option<TimeFilter> {
55+
TimeFilter::from_str(s).map(TimeFilter::Before)
4256
}
4357

44-
pub fn after(ref_time: &SystemTime, s: &str) -> Option<TimeFilter> {
45-
TimeFilter::from_str(ref_time, s).map(TimeFilter::After)
58+
pub fn after(s: &str) -> Option<TimeFilter> {
59+
TimeFilter::from_str(s).map(TimeFilter::After)
4660
}
4761

4862
pub fn applies_to(&self, t: &SystemTime) -> bool {
@@ -58,107 +72,96 @@ mod tests {
5872
use super::*;
5973
use std::time::Duration;
6074

75+
fn set_test_time(time: Zoned) -> SystemTime {
76+
TESTTIME.with_borrow_mut(|t| *t = Some(time.clone()));
77+
time.into()
78+
}
79+
6180
#[test]
6281
fn is_time_filter_applicable() {
6382
let local_tz = TimeZone::system();
64-
let ref_time = local_tz
65-
.to_ambiguous_zoned("2010-10-10 10:10:10".parse::<DateTime>().unwrap())
66-
.later()
67-
.unwrap()
68-
.into();
83+
let ref_time = set_test_time(
84+
local_tz
85+
.to_ambiguous_zoned("2010-10-10 10:10:10".parse::<DateTime>().unwrap())
86+
.later()
87+
.unwrap(),
88+
);
6989

70-
assert!(TimeFilter::after(&ref_time, "1min")
71-
.unwrap()
72-
.applies_to(&ref_time));
73-
assert!(!TimeFilter::before(&ref_time, "1min")
74-
.unwrap()
75-
.applies_to(&ref_time));
90+
assert!(TimeFilter::after("1min").unwrap().applies_to(&ref_time));
91+
assert!(!TimeFilter::before("1min").unwrap().applies_to(&ref_time));
7692

7793
let t1m_ago = ref_time - Duration::from_secs(60);
78-
assert!(!TimeFilter::after(&ref_time, "30sec")
79-
.unwrap()
80-
.applies_to(&t1m_ago));
81-
assert!(TimeFilter::after(&ref_time, "2min")
82-
.unwrap()
83-
.applies_to(&t1m_ago));
94+
assert!(!TimeFilter::after("30sec").unwrap().applies_to(&t1m_ago));
95+
assert!(TimeFilter::after("2min").unwrap().applies_to(&t1m_ago));
8496

85-
assert!(TimeFilter::before(&ref_time, "30sec")
86-
.unwrap()
87-
.applies_to(&t1m_ago));
88-
assert!(!TimeFilter::before(&ref_time, "2min")
89-
.unwrap()
90-
.applies_to(&t1m_ago));
97+
assert!(TimeFilter::before("30sec").unwrap().applies_to(&t1m_ago));
98+
assert!(!TimeFilter::before("2min").unwrap().applies_to(&t1m_ago));
9199

92100
let t10s_before = "2010-10-10 10:10:00";
93-
assert!(!TimeFilter::before(&ref_time, t10s_before)
101+
assert!(!TimeFilter::before(t10s_before)
94102
.unwrap()
95103
.applies_to(&ref_time));
96-
assert!(TimeFilter::before(&ref_time, t10s_before)
104+
assert!(TimeFilter::before(t10s_before)
97105
.unwrap()
98106
.applies_to(&t1m_ago));
99107

100-
assert!(TimeFilter::after(&ref_time, t10s_before)
108+
assert!(TimeFilter::after(t10s_before)
101109
.unwrap()
102110
.applies_to(&ref_time));
103-
assert!(!TimeFilter::after(&ref_time, t10s_before)
104-
.unwrap()
105-
.applies_to(&t1m_ago));
111+
assert!(!TimeFilter::after(t10s_before).unwrap().applies_to(&t1m_ago));
106112

107113
let same_day = "2010-10-10";
108-
assert!(!TimeFilter::before(&ref_time, same_day)
109-
.unwrap()
110-
.applies_to(&ref_time));
111-
assert!(!TimeFilter::before(&ref_time, same_day)
112-
.unwrap()
113-
.applies_to(&t1m_ago));
114-
115-
assert!(TimeFilter::after(&ref_time, same_day)
116-
.unwrap()
117-
.applies_to(&ref_time));
118-
assert!(TimeFilter::after(&ref_time, same_day)
119-
.unwrap()
120-
.applies_to(&t1m_ago));
121-
122-
let ref_time = "2010-10-10T10:10:10+00:00"
123-
.parse::<Timestamp>()
124-
.unwrap()
125-
.into();
114+
assert!(!TimeFilter::before(same_day).unwrap().applies_to(&ref_time));
115+
assert!(!TimeFilter::before(same_day).unwrap().applies_to(&t1m_ago));
116+
117+
assert!(TimeFilter::after(same_day).unwrap().applies_to(&ref_time));
118+
assert!(TimeFilter::after(same_day).unwrap().applies_to(&t1m_ago));
119+
120+
let ref_time = set_test_time(
121+
"2010-10-10T10:10:10+00:00"
122+
.parse::<Timestamp>()
123+
.unwrap()
124+
.to_zoned(local_tz.clone()),
125+
);
126126
let t1m_ago = ref_time - Duration::from_secs(60);
127127
let t10s_before = "2010-10-10T10:10:00+00:00";
128-
assert!(!TimeFilter::before(&ref_time, t10s_before)
128+
assert!(!TimeFilter::before(t10s_before)
129129
.unwrap()
130130
.applies_to(&ref_time));
131-
assert!(TimeFilter::before(&ref_time, t10s_before)
131+
assert!(TimeFilter::before(t10s_before)
132132
.unwrap()
133133
.applies_to(&t1m_ago));
134134

135-
assert!(TimeFilter::after(&ref_time, t10s_before)
135+
assert!(TimeFilter::after(t10s_before)
136136
.unwrap()
137137
.applies_to(&ref_time));
138-
assert!(!TimeFilter::after(&ref_time, t10s_before)
139-
.unwrap()
140-
.applies_to(&t1m_ago));
138+
assert!(!TimeFilter::after(t10s_before).unwrap().applies_to(&t1m_ago));
141139

142140
let ref_timestamp = 1707723412u64; // Mon Feb 12 07:36:52 UTC 2024
143-
let ref_time = "2024-02-12T07:36:52+00:00"
144-
.parse::<Timestamp>()
145-
.unwrap()
146-
.into();
141+
let ref_time = set_test_time(
142+
"2024-02-12T07:36:52+00:00"
143+
.parse::<Timestamp>()
144+
.unwrap()
145+
.to_zoned(local_tz),
146+
);
147147
let t1m_ago = ref_time - Duration::from_secs(60);
148148
let t1s_later = ref_time + Duration::from_secs(1);
149149
// Timestamp only supported via '@' prefix
150-
assert!(TimeFilter::before(&ref_time, &ref_timestamp.to_string()).is_none());
151-
assert!(TimeFilter::before(&ref_time, &format!("@{ref_timestamp}"))
150+
assert!(TimeFilter::before(&ref_timestamp.to_string()).is_none());
151+
assert!(TimeFilter::before(&format!("@{ref_timestamp}"))
152152
.unwrap()
153153
.applies_to(&t1m_ago));
154-
assert!(!TimeFilter::before(&ref_time, &format!("@{ref_timestamp}"))
154+
assert!(!TimeFilter::before(&format!("@{ref_timestamp}"))
155155
.unwrap()
156156
.applies_to(&t1s_later));
157-
assert!(!TimeFilter::after(&ref_time, &format!("@{ref_timestamp}"))
157+
assert!(!TimeFilter::after(&format!("@{ref_timestamp}"))
158158
.unwrap()
159159
.applies_to(&t1m_ago));
160-
assert!(TimeFilter::after(&ref_time, &format!("@{ref_timestamp}"))
160+
assert!(TimeFilter::after(&format!("@{ref_timestamp}"))
161161
.unwrap()
162162
.applies_to(&t1s_later));
163+
164+
// Stop using manually set times
165+
TESTTIME.with_borrow_mut(|t| *t = None);
163166
}
164167
}

‎src/main.rs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ use std::env;
1717
use std::io::IsTerminal;
1818
use std::path::Path;
1919
use std::sync::Arc;
20-
use std::time;
2120

2221
use anyhow::{anyhow, bail, Context, Result};
2322
use clap::{CommandFactory, Parser};
@@ -429,10 +428,9 @@ fn determine_ls_command(colored_output: bool) -> Result<Vec<&'static str>> {
429428
}
430429

431430
fn extract_time_constraints(opts: &Opts) -> Result<Vec<TimeFilter>> {
432-
let now = time::SystemTime::now();
433431
let mut time_constraints: Vec<TimeFilter> = Vec::new();
434432
if let Some(ref t) = opts.changed_within {
435-
if let Some(f) = TimeFilter::after(&now, t) {
433+
if let Some(f) = TimeFilter::after(t) {
436434
time_constraints.push(f);
437435
} else {
438436
return Err(anyhow!(
@@ -442,7 +440,7 @@ fn extract_time_constraints(opts: &Opts) -> Result<Vec<TimeFilter>> {
442440
}
443441
}
444442
if let Some(ref t) = opts.changed_before {
445-
if let Some(f) = TimeFilter::before(&now, t) {
443+
if let Some(f) = TimeFilter::before(t) {
446444
time_constraints.push(f);
447445
} else {
448446
return Err(anyhow!(

0 commit comments

Comments
 (0)
⚡