From 515a7bf9dc909fbfc996e633ced9128c0bfa5f17 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). Finally, we split win-gnu out to its own CI job and run a stripped down set of tests. Regretable, but it's *twice* as slow as the next slowest runner. That's sucks enough that we'll just live with worse test coverage until this becomes an obvious problem. (I'm also not even sure anyone is using this target anyway. It's not clear why anyone would.) --- .github/workflows/ci.yml | 27 +++++++++++++++++++++++---- Cargo.toml | 10 +++++++--- src/civil/date.rs | 21 ++++++--------------- src/timestamp.rs | 11 +++++++---- src/tz/tzif.rs | 10 ++++++++-- 5 files changed, 51 insertions(+), 28 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d9b9031..b6bafa4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,9 +49,6 @@ jobs: - build: win-msvc os: windows-latest rust: stable - - build: win-gnu - os: windows-latest - rust: stable-x86_64-gnu steps: - name: Checkout repository uses: actions/checkout@v4 @@ -64,9 +61,31 @@ 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' + - if: matrix.build != 'win-msvc' + run: cargo test --verbose --lib --profile testrelease + - if: matrix.build != 'win-msvc' + run: cargo test --verbose --test integration --profile testrelease + - if: matrix.build != 'win-msvc' run: ./test + # Tests for stable-x86_64-gnu. It's different enough from the "main" targets + # to warrant its own section. The main problem is that it is just + # annoyingly slow. Like, twice as slow as the next slowest runner. So we + # run a stripped down version of tests. + win-gnu: + runs-on: windows-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + - name: Install Rust + uses: dtolnay/rust-toolchain@master + with: + toolchain: stable-x86_64-gnu + - run: cargo build --verbose + - run: cargo doc --verbose + - run: cargo test --verbose --lib + - run: cargo test --verbose --test integration + # This job runs a stripped down version of CI to test the MSRV. The specific # reason for doing this is that Jiff dev-dependencies tend to evolve more # quickly. Or if I want to use newer features in doc examples. There isn't as 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) {