Skip to content

Commit

Permalink
test: disable optimizations when testing
Browse files Browse the repository at this point in the history
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.)
  • Loading branch information
BurntSushi committed Aug 3, 2024
1 parent 1a913e7 commit d5f5f45
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 28 deletions.
27 changes: 23 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 7 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 6 additions & 15 deletions src/civil/date.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down
11 changes: 7 additions & 4 deletions src/timestamp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
}
Expand Down
10 changes: 8 additions & 2 deletions src/tz/tzif.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand All @@ -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) {
Expand Down

0 comments on commit d5f5f45

Please sign in to comment.