chrono
chrono copied to clipboard
Add a feature and methods to override `now`
Support more testing use cases by adding a way to override the current
time that is returned by Utc::now()
and Local::now()
.
[package]
resolver = "2"
[dependencies]
chrono = "0.4"
[dev-dependencies]
chrono = { version = "0.4", features = ["test-override"] }
use chrono::{Utc, Datelike};
pub fn is_today_leap_day() -> bool {
let today = Utc::today();
today.month() == 2 && today.day() == 29
}
#[cfg(test)]
mod tests {
use super::is_today_leap_day;
use chrono::{Utc, DateTime};
#[test]
fn returns_true_on_leap_day() {
Utc::test_override_now(Utc.ymd(2020, 2, 29).and_hms(0, 0, 0));
assert!(is_today_leap_day());
}
#[test]
fn returns_false_on_other_days() {
Utc::test_override_now(Utc.ymd(2020, 2, 28).and_hms(0, 0, 0));
assert!(!is_today_leap_day());
Utc::test_override_now(Utc.ymd(2020, 3, 1).and_hms(0, 0, 0));
assert!(!is_today_leap_day());
}
}
Fixes #105
Thanks for contributing to chrono!
- [x] Have you added yourself and the change to the changelog? (Don't worry about adding the PR number)
I'm definitely open to thoughts on what the name of the feature should be and what the name of the methods should be.
I also considered returning another struct when overriding that will restore on drop. Something like
{
let o = Utc::test_override();
o.set(Utc.ymd(2020, 2, 29).and_hms(0, 0, 0));
assert!(Utc::now() < Utc.ymd(2021, 1, 1).and_hms(0, 0, 0));
}
assert!(Utc::now() > Utc.ymd(2021, 1, 1).and_hms(0, 0, 0));
but I thought that might add too much complexity, especially if Utc::test_override
was called multiple times.
The other thought on naming is to put the override functions in their own module instead of hanging them off of Utc
and Local
.
-
chrono::test_override::set_utc(_: DateTime<Utc>)
-
chrono::test_override::clear_utc()
-
chrono::test_override::set_local(_: DateTime<Local>)
-
chrono::test_override::clear_local()
use chrono::{Utc, Datelike};
pub fn is_today_leap_day() -> bool {
let today = Utc::today();
today.month() == 2 && today.day() == 29
}
#[cfg(test)]
mod tests {
use super::is_today_leap_day;
use chrono::{Utc, DateTime};
#[test]
fn returns_true_on_leap_day() {
chrono::test_override::set_utc(Utc.ymd(2020, 2, 29).and_hms(0, 0, 0));
assert!(is_today_leap_day());
}
#[test]
fn returns_false_on_other_days() {
chrono::test_override::set_utc(Utc.ymd(2020, 2, 29).and_hms(0, 0, 0));
assert!(!is_today_leap_day());
chrono::test_override::set_utc(Utc.ymd(2020, 2, 29).and_hms(0, 0, 0));
assert!(!is_today_leap_day());
}
}
Any progress on this?
I am working on some tests where I would need this. Is there anything I can do to help push this forward?
Hi @slashrsm and @bryanburgers - I can see where this is useful for testing, just a few thoughts:
- Could your use cases instead be solved by passing a
DateTime
to the relevant functions, and having the actual current time generated somewhere earlier in the code? - I'm a bit concerned about this being enabled by a dependency and then causing unexpected effects, potentially one way to resolve this could be changing the cfg directive from
#[cfg(feature = "test-override")]
to#[cfg(all(test, feature = "test-override"))]
. Another (or additional) option could be making theset
andclear
functions private and having an API that runs the fixed now time on a separate thread to avoid interference with other code
I think this would have to come with a very strong rationale for why (something like) this is the best possible design. I don't think it fits very well with the scope of chrono as a fairly low-level library.
I think this would have to come with a very strong rationale for why (something like) this is the best possible design. I don't think it fits very well with the scope of chrono as a fairly low-level library.
I personally don't have any strong rationale to offer. I can imagine there are integration tests and unfortunately structured codebases where adjusting the current time is the easiest or even only way to do some tests.
However https://github.com/chronotope/chrono/issues/105#issuecomment-633260201 in the related issue has collected 28 thumb ups in three years. This may be our most popular feature request.
To comment on this PR:
I would add just one standalone public function in the offset
module, which can serve to set and clear the override:
pub fn override_now(datetime: Option<DateTime<FixedOffset>>);
And by making it take a FixedOffset
you can also use it to test different timezone offsets.
I also like the suggestion by @esheppa in https://github.com/chronotope/chrono/pull/580#issuecomment-1173780170 to confine the feature to testing only with #[cfg(all(test, feature = "test-override"))]
.
@bryanburgers would you still be available to work on this after two years?
I personally don't have any strong rationale to offer. I can imagine there are integration tests and unfortunately strutured codebases where adjusting the current time is the easiest or even only way to do some tests.
Yes, I have that exact scenarios, and it's not that uncommon. You're writing a test looking at data fixtures coming originally from an outside service, and your code is time related to now()
. For example checking if more than a time period has passed, or should that session be expired or not.
To chime in "as a Test Engineer", I think the overall idea here is sound.
Skimming the code changes, they seem okay to me. However, I can't comment if this is necessarily the cleanest way to do this; would require a long think.
Additionally, I couldn't determine if the override of now
behavior is being used for all the test cases that call now
, which is what this PR should also do (if it's not already). At first glance, it appears that is not occurring.
The way I do it so far is having an app context Context
and use ctx.now()
everywhere I need:
#[derive(Clone)]
pub struct AppContext {
pub now: Option<DateTime<Utc>>,
}
impl AppContext {
pub fn now(&self) -> DateTime<Utc> {
self.now.unwrap_or(chrono::Utc::now())
}
pub fn set_now(&mut self, now: &str) {
self.now = Some(now.parse().expect("Can't parse now"));
}
}
@jtmoon79 I planned on taking over this PR next week, but maybe it is more your cup of tea?
@jtmoon79 I planned on taking over this PR next week, but maybe it is more your cup of tea?
Before doing any work, I'll wait until @djc approves of the concept.
I guess I'm open to having something like this. I think it should default to off and should only be turned on with a cfg
flag (not a Cargo feature, which can be turned on by indirect dependencies). We should offer a way both to set the current time and the timezone/offset.
https://github.com/chronotope/chrono/pull/1244 is the updated PR.