From eb7b60e74ea8e2a2b7dd45d6e6012d35fa53e020 Mon Sep 17 00:00:00 2001
From: Jacob Pratt <jacob@jhpratt.dev>
Date: Wed, 22 Feb 2023 03:12:14 -0500
Subject: [PATCH] Always test local offset

The reproduction requires tens of thousands of environment variables to
be set. Testing sets at most a handful. Let's try our luck.
---
 tests/main.rs                       |  8 +++++++-
 tests/offset_date_time.rs           | 15 ++++++++++-----
 tests/utc_offset.rs                 | 29 +++++++++++++++++++++--------
 tests/util.rs                       |  6 ++++--
 time/src/sys/local_offset_at/mod.rs |  7 ++++++-
 5 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/tests/main.rs b/tests/main.rs
index 7e09beaeb2..14b2066011 100644
--- a/tests/main.rs
+++ b/tests/main.rs
@@ -1,6 +1,7 @@
 #![deny(
     anonymous_parameters,
     clippy::all,
+    clippy::undocumented_unsafe_blocks,
     illegal_floating_point_literal_pattern,
     late_bound_lifetime_arguments,
     path_statements,
@@ -9,7 +10,6 @@
     trivial_casts,
     trivial_numeric_casts,
     unreachable_pub,
-    unsafe_code,
     unused_extern_crates
 )]
 #![warn(
@@ -101,6 +101,12 @@ macro_rules! require_all_features {
 }
 
 require_all_features! {
+    use std::sync::Mutex;
+
+    /// A lock to ensure that certain tests don't run in parallel, which could lead to a test
+    /// unexpectedly failing.
+    static SOUNDNESS_LOCK: Mutex<()> = Mutex::new(());
+
     /// Construct a non-exhaustive modifier.
     macro_rules! modifier {
         ($name:ident {
diff --git a/tests/offset_date_time.rs b/tests/offset_date_time.rs
index 707d1d8ade..f32705df38 100644
--- a/tests/offset_date_time.rs
+++ b/tests/offset_date_time.rs
@@ -11,14 +11,19 @@ fn now_utc() {
     assert_eq!(OffsetDateTime::now_utc().offset(), offset!(UTC));
 }
 
+#[cfg_attr(miri, ignore)]
 #[test]
 fn now_local() {
-    #[cfg(not(target_family = "unix"))]
-    assert!(OffsetDateTime::now_local().is_ok());
+    use time::util::local_offset::*;
+
+    let _guard = crate::SOUNDNESS_LOCK.lock().unwrap();
 
-    // Include for test coverage.
-    #[cfg(target_family = "unix")]
-    let _ = OffsetDateTime::now_local();
+    // Safety: Technically not sound. However, this is a test, and it's highly improbable that we
+    // will run into issues with setting an environment variable a few times.
+    unsafe { set_soundness(Soundness::Unsound) };
+    assert!(OffsetDateTime::now_local().is_ok());
+    // Safety: We're setting it back to sound.
+    unsafe { set_soundness(Soundness::Sound) };
 }
 
 #[test]
diff --git a/tests/utc_offset.rs b/tests/utc_offset.rs
index 670b262629..13e3eddedd 100644
--- a/tests/utc_offset.rs
+++ b/tests/utc_offset.rs
@@ -144,24 +144,37 @@ fn neg() {
     assert_eq!(-offset!(-23:59:59), offset!(+23:59:59));
 }
 
+#[cfg_attr(miri, ignore)]
 #[test]
 fn local_offset_at() {
-    #[cfg(not(target_family = "unix"))]
-    assert!(UtcOffset::local_offset_at(OffsetDateTime::UNIX_EPOCH).is_ok());
+    use time::util::local_offset::*;
+
+    let _guard = crate::SOUNDNESS_LOCK.lock().unwrap();
 
-    #[cfg(target_family = "unix")]
-    let _ = UtcOffset::local_offset_at(OffsetDateTime::UNIX_EPOCH);
+    // Safety: Technically not sound. However, this is a test, and it's highly improbable that we
+    // will run into issues with setting an environment variable a few times.
+    unsafe { set_soundness(Soundness::Unsound) };
+    assert!(UtcOffset::local_offset_at(OffsetDateTime::UNIX_EPOCH).is_ok());
+    // Safety: We're setting it back to sound.
+    unsafe { set_soundness(Soundness::Sound) };
 }
 
+#[cfg_attr(miri, ignore)]
 #[test]
 fn current_local_offset() {
-    #[cfg(not(target_family = "unix"))]
-    assert!(UtcOffset::current_local_offset().is_ok());
+    use time::util::local_offset::*;
+
+    let _guard = crate::SOUNDNESS_LOCK.lock().unwrap();
 
-    #[cfg(target_family = "unix")]
-    let _ = UtcOffset::current_local_offset();
+    // Safety: Technically not sound. However, this is a test, and it's highly improbable that we
+    // will run into issues with setting an environment variable a few times.
+    unsafe { set_soundness(Soundness::Unsound) };
+    assert!(UtcOffset::current_local_offset().is_ok());
+    // Safety: We're setting it back to sound.
+    unsafe { set_soundness(Soundness::Sound) };
 }
 
+// Note: This behavior is not guaranteed and will hopefully be changed in the future.
 #[test]
 #[cfg_attr(
     any(
diff --git a/tests/util.rs b/tests/util.rs
index c9de3b5802..dbb91847c7 100644
--- a/tests/util.rs
+++ b/tests/util.rs
@@ -77,16 +77,18 @@ fn weeks_in_year() {
     }
 }
 
-#[allow(unsafe_code)]
 #[test]
 fn local_offset_soundness() {
     use time::util::local_offset::*;
 
-    // Safety: cargo does not mutate the environment while tests are running.
+    let _guard = crate::SOUNDNESS_LOCK.lock().unwrap();
 
     assert_eq!(get_soundness(), Soundness::Sound);
+    // Safety: Technically not sound. However, this is a test, and it's highly improbable that we
+    // will run into issues with setting an environment variable a few times.
     unsafe { set_soundness(Soundness::Unsound) };
     assert_eq!(get_soundness(), Soundness::Unsound);
+    // Safety: We're setting it back to sound.
     unsafe { set_soundness(Soundness::Sound) };
     assert_eq!(get_soundness(), Soundness::Sound);
 }
diff --git a/time/src/sys/local_offset_at/mod.rs b/time/src/sys/local_offset_at/mod.rs
index 04cfffd5b2..3367ebb55c 100644
--- a/time/src/sys/local_offset_at/mod.rs
+++ b/time/src/sys/local_offset_at/mod.rs
@@ -19,5 +19,10 @@ use crate::{OffsetDateTime, UtcOffset};
 /// Attempt to obtain the system's UTC offset. If the offset cannot be determined, `None` is
 /// returned.
 pub(crate) fn local_offset_at(datetime: OffsetDateTime) -> Option<UtcOffset> {
-    imp::local_offset_at(datetime)
+    // miri does not support tzset()
+    if cfg!(miri) {
+        None
+    } else {
+        imp::local_offset_at(datetime)
+    }
 }