updated tests and files to make more test friendly

rewrite/src/main.rs

@@ -221,11 +221,28 @@ fn determine_location(args: &Args) -> Result<(Location, Config), Box<dyn std::er
             println!("Using location from command-line: {:.4}, {:.4}", loc.lat, loc.lon);
         }
 
-        // Still load config for other settings
+        // Load config for other settings
         let mut config = Config::load().unwrap_or_default();
-        // Update config with manual location
-        config.set_location(loc, LocationSource::Manual, None);
-        config.save().ok(); // Ignore save errors
+
+        // Only ask to save if running in interactive mode (not print, not one-shot)
+        if !args.print && !args.one_shot {
+            use dialoguer::Confirm;
+            let should_save = Confirm::new()
+                .with_prompt("Save this location for future use?")
+                .default(false)
+                .interact()
+                .unwrap_or(false);
+
+            if should_save {
+                config.set_location(loc, LocationSource::Manual, None);
+                config.save().ok(); // Ignore save errors
+                if args.verbose {
+                    println!("Location saved to configuration file.");
+                }
+            } else if args.verbose {
+                println!("Location will not be saved (session only).");
+            }
+        }
 
         return Ok((loc, config));
     }

rewrite/tests/cities_tests.rs

@@ -0,0 +1,296 @@
+/// Tests for cities module - location selection and data loading
+
+#[cfg(test)]
+mod cities_data_tests {
+    use redshift_rebooted::cities::*;
+
+    #[test]
+    fn test_cities_by_country_loaded() {
+        // Verify the lazy_static hash map is populated
+        assert!(!CITIES_BY_COUNTRY.is_empty(), "Cities should be loaded from JSON");
+        assert!(CITIES_BY_COUNTRY.len() > 100, "Should have many countries");
+    }
+
+    #[test]
+    fn test_get_countries_returns_sorted_list() {
+        let countries = get_countries();
+
+        assert!(!countries.is_empty(), "Should have countries");
+
+        // Verify sorting
+        for i in 1..countries.len() {
+            assert!(
+                countries[i - 1] <= countries[i],
+                "Countries should be sorted alphabetically"
+            );
+        }
+    }
+
+    #[test]
+    fn test_get_cities_for_existing_country() {
+        let countries = get_countries();
+
+        // Test a few countries that should exist
+        if countries.contains(&"United States".to_string()) {
+            let cities = get_cities_for_country("United States");
+            assert!(cities.is_some(), "United States should have cities");
+            assert!(!cities.unwrap().is_empty(), "United States should have at least one city");
+        }
+    }
+
+    #[test]
+    fn test_get_cities_for_nonexistent_country() {
+        let cities = get_cities_for_country("NonexistentCountry12345");
+        assert!(cities.is_none(), "Nonexistent country should return None");
+    }
+
+    #[test]
+    fn test_city_fields_are_populated() {
+        let countries = get_countries();
+        if let Some(country) = countries.first() {
+            if let Some(cities) = get_cities_for_country(country) {
+                if let Some(city) = cities.first() {
+                    // Verify all required fields are present
+                    assert!(!city.city.is_empty(), "City name should not be empty");
+                    assert!(!city.city_ascii.is_empty(), "City ASCII name should not be empty");
+                    assert!(!city.lat.is_empty(), "Latitude should not be empty");
+                    assert!(!city.lng.is_empty(), "Longitude should not be empty");
+                    assert!(!city.country.is_empty(), "Country should not be empty");
+                    assert!(!city.id.is_empty(), "ID should not be empty");
+
+                    // Verify coordinates can be parsed
+                    assert!(city.latitude().is_ok(), "Latitude should be parseable");
+                    assert!(city.longitude().is_ok(), "Longitude should be parseable");
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn test_city_coordinates_are_valid() {
+        let countries = get_countries();
+
+        for country in countries.iter().take(10) {
+            if let Some(cities) = get_cities_for_country(country) {
+                for city in cities.iter().take(5) {
+                    let lat = city.latitude().expect("Should parse latitude");
+                    let lon = city.longitude().expect("Should parse longitude");
+
+                    assert!(lat >= -90.0 && lat <= 90.0, "Latitude must be between -90 and 90");
+                    assert!(lon >= -180.0 && lon <= 180.0, "Longitude must be between -180 and 180");
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn test_city_state_flag() {
+        // Find a city-state if one exists
+        let countries = get_countries();
+
+        let mut found_city_state = false;
+        let mut found_non_city_state = false;
+
+        for country in countries.iter() {
+            if let Some(cities) = get_cities_for_country(country) {
+                if cities.len() == 1 && cities[0].is_city_country {
+                    found_city_state = true;
+                }
+                if cities.len() > 1 {
+                    for city in cities {
+                        if !city.is_city_country {
+                            found_non_city_state = true;
+                            break;
+                        }
+                    }
+                }
+            }
+        }
+
+        // We should have both types in our dataset
+        assert!(found_city_state || found_non_city_state,
+            "Should have at least some cities with is_city_country flags");
+    }
+}
+
+#[cfg(test)]
+mod city_display_tests {
+    use redshift_rebooted::cities::City;
+
+    fn create_test_city(city: &str, city_ascii: &str) -> City {
+        City {
+            city: city.to_string(),
+            city_ascii: city_ascii.to_string(),
+            lat: "40.7128".to_string(),
+            lng: "-74.0060".to_string(),
+            country: "Test Country".to_string(),
+            admin_name: "Test Admin".to_string(),
+            population: "1000000".to_string(),
+            id: "12345".to_string(),
+            is_city_country: false,
+        }
+    }
+
+    #[test]
+    fn test_display_name_same_city_and_ascii() {
+        let city = create_test_city("NewYork", "NewYork");
+        let display = city.display_name();
+
+        // When city and city_ascii are the same, should just show city_ascii
+        assert_eq!(display, "NewYork");
+    }
+
+    #[test]
+    fn test_city_latitude_parsing() {
+        let city = create_test_city("Test", "Test");
+        let lat = city.latitude();
+
+        assert!(lat.is_ok());
+        assert!((lat.unwrap() - 40.7128).abs() < 0.0001);
+    }
+
+    #[test]
+    fn test_city_longitude_parsing() {
+        let city = create_test_city("Test", "Test");
+        let lon = city.longitude();
+
+        assert!(lon.is_ok());
+        assert!((lon.unwrap() - (-74.0060)).abs() < 0.0001);
+    }
+
+    #[test]
+    fn test_city_invalid_coordinates() {
+        let mut city = create_test_city("Test", "Test");
+        city.lat = "invalid".to_string();
+
+        assert!(city.latitude().is_err());
+    }
+
+    #[test]
+    fn test_city_empty_coordinates() {
+        let mut city = create_test_city("Test", "Test");
+        city.lat = "".to_string();
+
+        assert!(city.latitude().is_err());
+    }
+}
+
+#[cfg(test)]
+mod utf8_locale_tests {
+    // Note: These tests can't easily test the actual UTF-8 locale detection
+    // since it depends on environment variables, but we can test the logic
+
+    #[test]
+    fn test_city_display_logic() {
+        // This is tested indirectly through the display_name tests above
+        // The actual UTF-8 detection happens at runtime based on LANG env var
+    }
+}
+
+#[cfg(test)]
+mod data_integrity_tests {
+    use redshift_rebooted::cities::*;
+    use std::collections::HashSet;
+
+    #[test]
+    fn test_no_duplicate_city_ids() {
+        let mut seen_ids = HashSet::new();
+        let countries = get_countries();
+
+        for country in countries {
+            if let Some(cities) = get_cities_for_country(&country) {
+                for city in cities {
+                    assert!(
+                        seen_ids.insert(city.id.clone()),
+                        "City ID {} appears more than once",
+                        city.id
+                    );
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn test_all_cities_have_matching_country() {
+        let countries = get_countries();
+
+        for country in countries {
+            if let Some(cities) = get_cities_for_country(&country) {
+                for city in cities {
+                    assert_eq!(
+                        city.country, country,
+                        "City {} has mismatched country field",
+                        city.city
+                    );
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn test_populations_are_numeric_or_empty() {
+        let countries = get_countries();
+
+        for country in countries.iter().take(20) {
+            if let Some(cities) = get_cities_for_country(country) {
+                for city in cities {
+                    if !city.population.is_empty() {
+                        // Try to parse as float to handle values like "7740.00"
+                        let parse_result = city.population.parse::<f64>();
+                        assert!(
+                            parse_result.is_ok(),
+                            "Population '{}' for city {} should be numeric",
+                            city.population,
+                            city.city
+                        );
+                    }
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn test_city_state_consistency() {
+        let countries = get_countries();
+
+        for country in countries {
+            if let Some(cities) = get_cities_for_country(&country) {
+                if cities.len() == 1 {
+                    // Single city countries should have is_city_country = true
+                    assert!(
+                        cities[0].is_city_country,
+                        "Country {} has only 1 city but is_city_country is false",
+                        country
+                    );
+                } else {
+                    // Multi-city countries should have is_city_country = false
+                    for city in cities {
+                        assert!(
+                            !city.is_city_country,
+                            "Country {} has multiple cities but {} has is_city_country = true",
+                            country,
+                            city.city
+                        );
+                    }
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn test_reasonable_population_sizes() {
+        let countries = get_countries();
+
+        for country in countries.iter().take(20) {
+            if let Some(cities) = get_cities_for_country(country) {
+                for city in cities {
+                    if let Ok(pop) = city.population.parse::<f64>() {
+                        // Population should be reasonable (not negative, not absurdly high)
+                        assert!(pop >= 0.0, "Population cannot be negative");
+                        assert!(pop < 100_000_000.0, "City population seems unreasonably high");
+                    }
+                }
+            }
+        }
+    }
+}

rewrite/tests/main_tests.rs

@@ -0,0 +1,472 @@
+/// Tests for main.rs location parsing and determination logic
+
+use redshift_rebooted::types::*;
+
+// Helper to parse location string (mimics main.rs parse_location)
+fn parse_location(loc_str: &str) -> Result<Location, String> {
+    let parts: Vec<&str> = loc_str.split(':').collect();
+    if parts.len() != 2 {
+        return Err("Location must be in format LAT:LON".to_string());
+    }
+
+    let lat: f32 = parts[0]
+        .parse()
+        .map_err(|_| format!("Invalid latitude: {}", parts[0]))?;
+    let lon: f32 = parts[1]
+        .parse()
+        .map_err(|_| format!("Invalid longitude: {}", parts[1]))?;
+
+    if lat < MIN_LAT || lat > MAX_LAT {
+        return Err(format!(
+            "Latitude must be between {} and {}",
+            MIN_LAT, MAX_LAT
+        ));
+    }
+    if lon < MIN_LON || lon > MAX_LON {
+        return Err(format!(
+            "Longitude must be between {} and {}",
+            MIN_LON, MAX_LON
+        ));
+    }
+
+    Ok(Location { lat, lon })
+}
+
+#[cfg(test)]
+mod parse_location_tests {
+    use super::*;
+
+    #[test]
+    fn test_parse_valid_location_positive() {
+        let result = parse_location("40.7:-74.0");
+        assert!(result.is_ok());
+        let loc = result.unwrap();
+        assert_eq!(loc.lat, 40.7);
+        assert_eq!(loc.lon, -74.0);
+    }
+
+    #[test]
+    fn test_parse_valid_location_negative() {
+        let result = parse_location("-33.9:151.2");
+        assert!(result.is_ok());
+        let loc = result.unwrap();
+        assert_eq!(loc.lat, -33.9);
+        assert_eq!(loc.lon, 151.2);
+    }
+
+    #[test]
+    fn test_parse_valid_location_zero() {
+        let result = parse_location("0:0");
+        assert!(result.is_ok());
+        let loc = result.unwrap();
+        assert_eq!(loc.lat, 0.0);
+        assert_eq!(loc.lon, 0.0);
+    }
+
+    #[test]
+    fn test_parse_location_boundary_values() {
+        // Test max latitude
+        let result = parse_location("90:0");
+        assert!(result.is_ok());
+
+        // Test min latitude
+        let result = parse_location("-90:0");
+        assert!(result.is_ok());
+
+        // Test max longitude
+        let result = parse_location("0:180");
+        assert!(result.is_ok());
+
+        // Test min longitude
+        let result = parse_location("0:-180");
+        assert!(result.is_ok());
+    }
+
+    #[test]
+    fn test_parse_location_invalid_format_no_colon() {
+        let result = parse_location("40.7");
+        assert!(result.is_err());
+        assert!(result.unwrap_err().contains("LAT:LON"));
+    }
+
+    #[test]
+    fn test_parse_location_invalid_format_too_many_colons() {
+        let result = parse_location("40:74:0");
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn test_parse_location_invalid_latitude_not_a_number() {
+        let result = parse_location("abc:74.0");
+        assert!(result.is_err());
+        assert!(result.unwrap_err().contains("Invalid latitude"));
+    }
+
+    #[test]
+    fn test_parse_location_invalid_longitude_not_a_number() {
+        let result = parse_location("40.7:xyz");
+        assert!(result.is_err());
+        assert!(result.unwrap_err().contains("Invalid longitude"));
+    }
+
+    #[test]
+    fn test_parse_location_latitude_too_high() {
+        let result = parse_location("91:0");
+        assert!(result.is_err());
+        assert!(result.unwrap_err().contains("Latitude must be between"));
+    }
+
+    #[test]
+    fn test_parse_location_latitude_too_low() {
+        let result = parse_location("-91:0");
+        assert!(result.is_err());
+        assert!(result.unwrap_err().contains("Latitude must be between"));
+    }
+
+    #[test]
+    fn test_parse_location_longitude_too_high() {
+        let result = parse_location("0:181");
+        assert!(result.is_err());
+        assert!(result.unwrap_err().contains("Longitude must be between"));
+    }
+
+    #[test]
+    fn test_parse_location_longitude_too_low() {
+        let result = parse_location("0:-181");
+        assert!(result.is_err());
+        assert!(result.unwrap_err().contains("Longitude must be between"));
+    }
+
+    #[test]
+    fn test_parse_location_with_decimal_precision() {
+        let result = parse_location("40.7128:-74.0060");
+        assert!(result.is_ok());
+        let loc = result.unwrap();
+        assert!((loc.lat - 40.7128).abs() < 0.0001);
+        assert!((loc.lon - (-74.0060)).abs() < 0.0001);
+    }
+
+    #[test]
+    fn test_parse_location_empty_string() {
+        let result = parse_location("");
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn test_parse_location_only_colon() {
+        let result = parse_location(":");
+        assert!(result.is_err());
+    }
+}
+
+#[cfg(test)]
+mod interpolation_tests {
+    use super::*;
+
+    fn interpolate_color_setting(
+        elevation: f64,
+        low: f64,
+        high: f64,
+        night: &ColorSetting,
+        day: &ColorSetting,
+    ) -> ColorSetting {
+        let alpha = ((elevation - low) / (high - low)) as f32;
+        let alpha = alpha.max(0.0).min(1.0);
+
+        ColorSetting {
+            temperature: ((1.0 - alpha) * (night.temperature as f32) + alpha * (day.temperature as f32))
+                as i32,
+            gamma: [
+                (1.0 - alpha) * night.gamma[0] + alpha * day.gamma[0],
+                (1.0 - alpha) * night.gamma[1] + alpha * day.gamma[1],
+                (1.0 - alpha) * night.gamma[2] + alpha * day.gamma[2],
+            ],
+            brightness: (1.0 - alpha) * night.brightness + alpha * day.brightness,
+        }
+    }
+
+    #[test]
+    fn test_interpolate_at_low_elevation() {
+        let night = ColorSetting {
+            temperature: 3500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+        let day = ColorSetting {
+            temperature: 6500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        let result = interpolate_color_setting(-6.0, -6.0, 3.0, &night, &day);
+        assert_eq!(result.temperature, 3500);
+    }
+
+    #[test]
+    fn test_interpolate_at_high_elevation() {
+        let night = ColorSetting {
+            temperature: 3500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+        let day = ColorSetting {
+            temperature: 6500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        let result = interpolate_color_setting(3.0, -6.0, 3.0, &night, &day);
+        assert_eq!(result.temperature, 6500);
+    }
+
+    #[test]
+    fn test_interpolate_midpoint() {
+        let night = ColorSetting {
+            temperature: 3500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+        let day = ColorSetting {
+            temperature: 6500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        let result = interpolate_color_setting(-1.5, -6.0, 3.0, &night, &day);
+        // Midpoint between 3500 and 6500 is 5000
+        assert_eq!(result.temperature, 5000);
+    }
+
+    #[test]
+    fn test_interpolate_brightness() {
+        let night = ColorSetting {
+            temperature: 3500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 0.5,
+        };
+        let day = ColorSetting {
+            temperature: 6500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        let result = interpolate_color_setting(-1.5, -6.0, 3.0, &night, &day);
+        assert!((result.brightness - 0.75).abs() < 0.01);
+    }
+
+    #[test]
+    fn test_interpolate_gamma() {
+        let night = ColorSetting {
+            temperature: 3500,
+            gamma: [0.8, 0.8, 0.8],
+            brightness: 1.0,
+        };
+        let day = ColorSetting {
+            temperature: 6500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        let result = interpolate_color_setting(-1.5, -6.0, 3.0, &night, &day);
+        assert!((result.gamma[0] - 0.9).abs() < 0.01);
+        assert!((result.gamma[1] - 0.9).abs() < 0.01);
+        assert!((result.gamma[2] - 0.9).abs() < 0.01);
+    }
+
+    #[test]
+    fn test_interpolate_below_range_clamps() {
+        let night = ColorSetting {
+            temperature: 3500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+        let day = ColorSetting {
+            temperature: 6500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        // Elevation below low should clamp to night
+        let result = interpolate_color_setting(-10.0, -6.0, 3.0, &night, &day);
+        assert_eq!(result.temperature, 3500);
+    }
+
+    #[test]
+    fn test_interpolate_above_range_clamps() {
+        let night = ColorSetting {
+            temperature: 3500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+        let day = ColorSetting {
+            temperature: 6500,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        // Elevation above high should clamp to day
+        let result = interpolate_color_setting(10.0, -6.0, 3.0, &night, &day);
+        assert_eq!(result.temperature, 6500);
+    }
+}
+
+#[cfg(test)]
+mod color_setting_tests {
+    use super::*;
+
+    fn color_setting_diff_is_major(first: &ColorSetting, second: &ColorSetting) -> bool {
+        (first.temperature - second.temperature).abs() > 25
+            || (first.brightness - second.brightness).abs() > 0.1
+            || (first.gamma[0] - second.gamma[0]).abs() > 0.1
+            || (first.gamma[1] - second.gamma[1]).abs() > 0.1
+            || (first.gamma[2] - second.gamma[2]).abs() > 0.1
+    }
+
+    #[test]
+    fn test_identical_settings_not_major() {
+        let setting = ColorSetting {
+            temperature: 5000,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        assert!(!color_setting_diff_is_major(&setting, &setting));
+    }
+
+    #[test]
+    fn test_small_temperature_diff_not_major() {
+        let first = ColorSetting {
+            temperature: 5000,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+        let second = ColorSetting {
+            temperature: 5020,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        assert!(!color_setting_diff_is_major(&first, &second));
+    }
+
+    #[test]
+    fn test_large_temperature_diff_is_major() {
+        let first = ColorSetting {
+            temperature: 5000,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+        let second = ColorSetting {
+            temperature: 5100,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        assert!(color_setting_diff_is_major(&first, &second));
+    }
+
+    #[test]
+    fn test_brightness_diff_is_major() {
+        let first = ColorSetting {
+            temperature: 5000,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+        let second = ColorSetting {
+            temperature: 5000,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 0.8,
+        };
+
+        assert!(color_setting_diff_is_major(&first, &second));
+    }
+
+    #[test]
+    fn test_gamma_diff_is_major() {
+        let first = ColorSetting {
+            temperature: 5000,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+        let second = ColorSetting {
+            temperature: 5000,
+            gamma: [0.85, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        assert!(color_setting_diff_is_major(&first, &second));
+    }
+
+    #[test]
+    fn test_boundary_temperature_25k_not_major() {
+        let first = ColorSetting {
+            temperature: 5000,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+        let second = ColorSetting {
+            temperature: 5025,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        assert!(!color_setting_diff_is_major(&first, &second));
+    }
+
+    #[test]
+    fn test_boundary_temperature_26k_is_major() {
+        let first = ColorSetting {
+            temperature: 5000,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+        let second = ColorSetting {
+            temperature: 5026,
+            gamma: [1.0, 1.0, 1.0],
+            brightness: 1.0,
+        };
+
+        assert!(color_setting_diff_is_major(&first, &second));
+    }
+}
+
+#[cfg(test)]
+mod ease_fade_tests {
+    fn ease_fade(t: f64) -> f64 {
+        t * t * (3.0 - 2.0 * t)
+    }
+
+    #[test]
+    fn test_ease_fade_at_zero() {
+        assert_eq!(ease_fade(0.0), 0.0);
+    }
+
+    #[test]
+    fn test_ease_fade_at_one() {
+        assert_eq!(ease_fade(1.0), 1.0);
+    }
+
+    #[test]
+    fn test_ease_fade_at_half() {
+        let result = ease_fade(0.5);
+        assert!((result - 0.5).abs() < 0.0001);
+    }
+
+    #[test]
+    fn test_ease_fade_smooth_curve() {
+        // Ease function should be smooth (no sudden jumps)
+        let t1 = ease_fade(0.3);
+        let t2 = ease_fade(0.31);
+        assert!((t2 - t1).abs() < 0.1); // Should change gradually
+    }
+
+    #[test]
+    fn test_ease_fade_monotonic_increasing() {
+        // Function should always increase
+        for i in 0..100 {
+            let t1 = i as f64 / 100.0;
+            let t2 = (i + 1) as f64 / 100.0;
+            assert!(ease_fade(t2) >= ease_fade(t1));
+        }
+    }
+}