From 8e043fa3934ca28338982c63021c29d336b57ff5 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Sat, 3 Aug 2024 18:40:01 -0400 Subject: [PATCH] test: disable optimizations when testing I was getting pretty tired of the long compile times in between test runs. I thought they seemed particularly long for debug mode. It turns out that I had actually set `opt-level = 3` for running tests. This was because there were a select few tests that ran quite long in debug mode. But iteration time is important, so I trimmed down those tests and disabled optimizations. Tests run much more quickly now after making a change to the source code. We also add `cargo test --profile testrelease` to CI. This is just like normal tests, but disables `debug_assertions`. Jiff has a lot of extra stuff going on when `debug_assertions` are enabled due to its internal ranged integer abstraction. So it's useful to run tests without all that extra stuff too (reflecting what is intended to be run in production). --- .github/workflows/ci.yml | 4 ++++ Cargo.toml | 10 +++++++--- src/civil/date.rs | 21 ++++++--------------- src/timestamp.rs | 11 +++++++---- src/tz/tzif.rs | 10 ++++++++-- 5 files changed, 32 insertions(+), 24 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d9b9031..5fa6aa4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,6 +64,10 @@ jobs: - run: cargo test --verbose --all - run: cargo test --verbose -p jiff-cli # Skip on Windows because it takes freaking forever. + - if: matrix.build != 'win-msvc' && matrix.build != 'win-gnu' + run: cargo test --verbose --lib --profile testrelease + - if: matrix.build != 'win-msvc' && matrix.build != 'win-gnu' + run: cargo test --verbose --test integration --profile testrelease - if: matrix.build != 'win-msvc' && matrix.build != 'win-gnu' run: ./test diff --git a/Cargo.toml b/Cargo.toml index 8e7fefa..204a07a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -120,9 +120,13 @@ version = "3.9.0" path = "tests/lib.rs" name = "integration" -[profile.test] -opt-level = 3 - +# This is just like the default 'test' profile, but debug_assertions are +# disabled. This is important to cover for Jiff because we do a lot of extra +# work in our internal ranged integer types when debug_assertions are enabled. +# It also makes types fatter. It's very useful for catching overflow bugs. +# But since there's a fair bit of logic there, it's also worth running tests +# without debug_assertions enabled to exercise the *actual* code paths used +# in production. [profile.testrelease] inherits = "test" debug-assertions = false diff --git a/src/civil/date.rs b/src/civil/date.rs index f1b8590..79d84fc 100644 --- a/src/civil/date.rs +++ b/src/civil/date.rs @@ -3606,7 +3606,7 @@ mod tests { #[test] fn all_days_to_date_roundtrip() { - for rd in UnixEpochDays::MIN_REPR..=UnixEpochDays::MAX_REPR { + for rd in -100_000..=100_000 { let rd = UnixEpochDays::new(rd).unwrap(); let date = Date::from_unix_epoch_days(rd); let got = date.to_unix_epoch_days(); @@ -3618,7 +3618,8 @@ mod tests { fn all_date_to_days_roundtrip() { use crate::util::common::days_in_month; - for year in Year::MIN_REPR..=Year::MAX_REPR { + let year_range = 2000..=2500; + for year in year_range { let year = Year::new(year).unwrap(); for month in Month::MIN_REPR..=Month::MAX_REPR { let month = Month::new(month).unwrap(); @@ -3636,22 +3637,12 @@ mod tests { fn all_date_to_iso_week_date_roundtrip() { use crate::util::common::days_in_month; - // This test is slow enough in debug mode to be worth tweaking a bit. - // We still want to run it so that we benefit from ranged integer - // checks, but we just do it for ~1000 years. We do it for at least 400 - // years to capture a single Gregorian cycle, and we also include the - // upper boundary of years because there is some special cased logic - // for dealing with that specific boundary condition. - let year_range = if cfg!(debug_assertions) { - 9000..=9999 - } else { - Year::MIN_REPR..=Year::MAX_REPR - }; + let year_range = 2000..=2500; for year in year_range { let year = Year::new(year).unwrap(); - for month in Month::MIN_REPR..=Month::MAX_REPR { + for month in [1, 2, 4] { let month = Month::new(month).unwrap(); - for day in 1..=days_in_month(year, month).get() { + for day in 20..=days_in_month(year, month).get() { let date = Date::constant(year.get(), month.get(), day); let wd = date.to_iso_week_date(); let got = wd.to_date(); diff --git a/src/timestamp.rs b/src/timestamp.rs index a0dbb69..5d13ec7 100644 --- a/src/timestamp.rs +++ b/src/timestamp.rs @@ -3057,7 +3057,7 @@ mod tests { } #[test] - fn to_datetime_every_second_in_some_days() { + fn to_datetime_many_seconds_in_some_days() { let days = [ i64::from(t::UnixEpochDays::MIN_REPR), -1000, @@ -3066,12 +3066,15 @@ mod tests { 2000, i64::from(t::UnixEpochDays::MAX_REPR), ]; + let seconds = [ + -86_400, -10, -9, -8, -7, -6, -5, -4, -3, -2, -1, 0, 1, 2, 3, 4, + 5, 6, 7, 8, 9, 10, 86_400, + ]; let nanos = [0, 1, 5, 999_999_999]; for day in days { let midpoint = day * 86_400; - let start = midpoint - 86_400; - let end = midpoint + 86_400; - for second in start..=end { + for second in seconds { + let second = midpoint + second; if !UnixSeconds::contains(second) { continue; } diff --git a/src/tz/tzif.rs b/src/tz/tzif.rs index f172b7a..8990e72 100644 --- a/src/tz/tzif.rs +++ b/src/tz/tzif.rs @@ -1478,8 +1478,8 @@ mod tests { } /// This tests walks the /usr/share/zoneinfo directory (if it exists) and - /// tries to parse an TZif formatted file it can find. We don't really do - /// much with it other than to ensure we don't panic or return an error. + /// tries to parse every TZif formatted file it can find. We don't really + /// do much with it other than to ensure we don't panic or return an error. /// That is, we check that we can parse each file, but not that we do so /// correctly. #[cfg(target_os = "linux")] @@ -1492,6 +1492,12 @@ mod tests { // These aren't related to our parsing, so it's some other problem // (like the directory not existing). let Ok(dent) = result else { continue }; + // This test can take some time in debug mode, so skip parsing + // some of the less frequently used TZif files. + let Some(name) = dent.path().to_str() else { continue }; + if name.contains("right/") || name.contains("posix/") { + continue; + } // Again, skip if we can't read. Not my monkeys, not my circus. let Ok(bytes) = std::fs::read(dent.path()) else { continue }; if !is_possibly_tzif(&bytes) {