From 0c76e77866f5c9881c5a9b042a176908839f8d48 Mon Sep 17 00:00:00 2001 From: hwanjjang Date: Sat, 7 Nov 2026 21:36:50 +0900 Subject: [PATCH 1/2] server2: make the province test independent of the zone list's arrival full_response::a_province_under_its_new_name_finds_its_town_and_mid_forecast failed intermittently, not at a time of day. The request starts the process's forecast-zone acquisition without waiting for it and then reads the stored lists (S3Source::zones/refresh_zones): the release's land zones when none is stored, the stored list otherwise. The test double answered the Seoul-only zone_page fixture, so whenever its publication won that race the towns outside Seoul had no city zone, their mid temperatures were not collected and the status said temperature-unavailable. With the wall clock forced to morning and evening starts, master failed 1 to 3 times in 20 at both. #2872 (from b7b81b5a, which reads the day products at the request's start) fails about 60% at any forced time (12:00 and 20:15 interleaved: 23/40 and 25/40): more request-lane reads before the zone read, not a clock read. The test now runs every town with the list stored before the request and without it, and the double answers the release's land zones besides the Seoul fixture, as the provider's list holds them. Test-only change. Co-Authored-By: Claude Opus 5.5 --- server2/config/tasks/KMA-TEST-CLOCK.json | 13 ++++++ server2/tests/domestic/full_response.rs | 59 ++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 server2/config/tasks/KMA-TEST-CLOCK.json diff --git a/server2/config/tasks/KMA-TEST-CLOCK.json b/server2/config/tasks/KMA-TEST-CLOCK.json new file mode 100644 index 000000000..d35df8ff1 --- /dev/null +++ b/server2/config/tasks/KMA-TEST-CLOCK.json @@ -0,0 +1,13 @@ +{ + "task": "KMA-TEST-CLOCK", + "issue": "https://github.com/WizardFactory/TodayWeather/issues/2614", + "scope": "Test-only: the province test of the full KMA response answered a Seoul-only forecast-zone list from its double, so its mid temperatures depended on whether that list was stored before the request's zone read. The test now runs both orders and the double answers the release's land zones. No runtime, deployment or production change.", + "server2_paths": [ + "server2/config/tasks/KMA-TEST-CLOCK.json", + "server2/tests/domestic/full_response.rs" + ], + "outside": [], + "path_notes": { + "server2/tests/domestic/full_response.rs": "a_province_under_its_new_name_finds_its_town_and_mid_forecast runs each town with the forecast-zone list stored before the request and without it; the Forecasts double's forecast-zone answer (land_zone_page) holds the Seoul fixture and every land zone of Zones::packaged, as the provider's list does" + } +} diff --git a/server2/tests/domestic/full_response.rs b/server2/tests/domestic/full_response.rs index 3949aef61..d108655ea 100644 --- a/server2/tests/domestic/full_response.rs +++ b/server2/tests/domestic/full_response.rs @@ -235,7 +235,7 @@ impl ForecastAcquire for Forecasts { }, p, p.base_date(), - zone_page(), + land_zone_page(), "application/json", ) } @@ -259,6 +259,39 @@ impl ForecastAcquire for Forecasts { .map(|r| r.records.clone()) } } +/// The forecast-zone list the double answers: like the provider's, it holds every land zone the +/// release carries (`Zones::packaged`, the city zones of `getMidTa`) besides the Seoul region of +/// [`zone_page`]. A request reads this list when it is stored before its zone read and the +/// release's land zones otherwise, so a test of a town outside Seoul answers the same either way +/// (a Seoul-only list left the mid temperatures of such towns uncollected whenever it was stored +/// first). +fn land_zone_page() -> Vec { + use server2::domestic::kma::source::{Val, Zones}; + let seoul: serde_json::Value = serde_json::from_slice(&zone_page()).unwrap(); + let mut items: Vec = seoul["response"]["body"]["items"]["item"] + .as_array() + .unwrap() + .clone(); + for zone in Zones::packaged().docs() { + let text = |k: &str| zone.get(k).and_then(Val::as_str).unwrap().to_owned(); + let id = text("regId"); + if items.iter().any(|item| item["regId"] == id.as_str()) { + continue; + } + items.push(json!({ + "regId": id, + "tmSt": "201601010000", + "tmEd": "210012310000", + "regSp": text("regSp"), + "regName": text("regName"), + })); + } + serde_json::to_vec(&json!({"response": { + "header": {"resultCode": "00", "resultMsg": "NORMAL_SERVICE"}, + "body": {"dataType": "JSON", "items": {"item": items}, "pageNo": 1, "numOfRows": 1000, + "totalCount": items.len()}}})) + .unwrap() +} struct FixedGeo; impl Geocoder for FixedGeo { fn coord(&self, _: [f64; 2], _: String) -> BoxFuture<'_, Result> { @@ -1214,20 +1247,38 @@ async fn a_town_outside_the_list_gets_its_forecast_within_the_bound() { } /// The geocoder's name for the province must reach the listed town and its mid forecast. +/// +/// The request starts the process's forecast-zone acquisition without waiting for it, and its +/// zone read then uses that list when it is already stored, the release's land zones otherwise. +/// Both orders happen (they depend on how fast the stored list is published), so the test runs +/// each town with the list stored before the request and without it. #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn a_province_under_its_new_name_finds_its_town_and_mid_forecast() { let _peers = S3_PEERS.lock().await; - for (region, city, town) in [ + for ((region, city, town), stored_first) in [ ("강원특별자치도", "강릉시", "홍제동"), ("전북특별자치도", "군산시", "조촌동"), ("전남광주통합특별시", "여수시", "쌍봉동"), ("전남광주통합특별시", "광산구", "송정2동"), - ] { + ] + .into_iter() + .flat_map(|town| [(town, true), (town, false)]) + { let peer = Peer::persistent(); let store = Arc::new( CatalogStore::new(peer.transport_limited(3), CatalogLimits::default()).unwrap(), ); let writer = Writer::new(store, Config::default()).unwrap(); + if stored_first { + Forecasts { + writer: writer.clone(), + calls: Arc::new(AtomicUsize::new(0)), + } + .acquire(ForecastRequest::Zones) + .await + .unwrap() + .expect("the forecast-zone list is stored"); + } let source = build_source(&peer, Arc::new(Providers::default())) .with_forecast(Some(Arc::new(Forecasts { writer: writer.clone(), @@ -1265,7 +1316,7 @@ async fn a_province_under_its_new_name_finds_its_town_and_mid_forecast() { "future-mid-unavailable" ] .contains(&reason.as_str().unwrap()), - "{region} {city}: {status}" + "{region} {city} (zone list stored first: {stored_first}): {status}" ); } let daily = body["midData"]["dailyData"].as_array().unwrap(); From 64541ef9b7c75357c9770cd27363220370d1094e Mon Sep 17 00:00:00 2001 From: hwanjjang Date: Sat, 7 Nov 2026 23:39:05 +0900 Subject: [PATCH 2/2] server2: hold the zone request in the province test's fallback case (TC-force-fallback-order) full_response::a_province_under_its_new_name_finds_its_town_and_mid_forecast asked each town with the forecast-zone list stored first and "without it", but nothing kept the request's own zone acquisition from storing the list before the answer: counting the lists S3 held when the answer was made, the "without" case had one in 10 of 10 runs, so it did not pin the release's land-zone fallback. In that case the double's zone request now waits at a barrier (HeldZones) until the answer has been checked, and the test asserts the lists S3 held at the answer (none, or the one stored first) and that the held request stores its list once released. 10/10 passes; with the packaged fallback removed it fails in that case (temperature-unavailable). Co-Authored-By: Claude Opus 5.5 --- server2/tests/domestic/full_response.rs | 76 +++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 6 deletions(-) diff --git a/server2/tests/domestic/full_response.rs b/server2/tests/domestic/full_response.rs index d108655ea..aa5bf6c10 100644 --- a/server2/tests/domestic/full_response.rs +++ b/server2/tests/domestic/full_response.rs @@ -1246,12 +1246,53 @@ async fn a_town_outside_the_list_gets_its_forecast_within_the_bound() { ); } +/// The test double's forecast-zone request held until [`HeldZones::release`]; every other request +/// goes straight to [`Forecasts`]. +struct HeldZones { + inner: Forecasts, + open: tokio::sync::watch::Sender, +} +impl HeldZones { + fn release(&self) { + self.open.send_replace(true); + } +} +impl ForecastAcquire for HeldZones { + fn acquire(&self, request: ForecastRequest) -> BoxFuture<'static, Result> { + let mut open = self.open.subscribe(); + let zones = request == ForecastRequest::Zones; + let acquired = self.inner.acquire(request); + Box::pin(async move { + if zones { + let _ = open.wait_for(|open| *open).await; + } + acquired.await + }) + } + fn cached(&self, request: &ForecastRequest) -> Published { + self.inner.cached(request) + } +} +/// The forecast-zone lists S3 holds now. +async fn stored_zone_lists(peer: &Peer) -> usize { + server2::history::IndexListing::list_index( + &peer.transport_limited(2), + "index/v2/data-go-kr/forecast-zone/", + None, + ) + .await + .unwrap() + .keys + .len() +} + /// The geocoder's name for the province must reach the listed town and its mid forecast. /// /// The request starts the process's forecast-zone acquisition without waiting for it, and its -/// zone read then uses that list when it is already stored, the release's land zones otherwise. -/// Both orders happen (they depend on how fast the stored list is published), so the test runs -/// each town with the list stored before the request and without it. +/// zone read then uses the stored list when there is one, the release's land zones otherwise. +/// Each town is asked both ways: with the list stored before the request, and with the zone +/// request held until the answer is checked, so that no list is stored before it and the land +/// zones answer (TC-force-fallback-order). #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn a_province_under_its_new_name_finds_its_town_and_mid_forecast() { let _peers = S3_PEERS.lock().await; @@ -1279,11 +1320,15 @@ async fn a_province_under_its_new_name_finds_its_town_and_mid_forecast() { .unwrap() .expect("the forecast-zone list is stored"); } - let source = build_source(&peer, Arc::new(Providers::default())) - .with_forecast(Some(Arc::new(Forecasts { + let zones = Arc::new(HeldZones { + inner: Forecasts { writer: writer.clone(), calls: Arc::new(AtomicUsize::new(0)), - }))) + }, + open: tokio::sync::watch::channel(stored_first).0, + }); + let source = build_source(&peer, Arc::new(Providers::default())) + .with_forecast(Some(zones.clone())) .with_raw_writer(writer.clone()); let backend = KmaBackend::new( Arc::new(source), @@ -1298,6 +1343,12 @@ async fn a_province_under_its_new_name_finds_its_town_and_mid_forecast() { .outcome(request, Instant::now() + Duration::from_secs(8)) .await .unwrap(); + // The lists the answer could read: the held request stored none before it. + assert_eq!( + stored_zone_lists(&peer).await, + usize::from(stored_first), + "{region} {city}: zone lists stored at the answer (stored first: {stored_first})" + ); let server2::domestic::kma::Outcome::Json(body) = outcome else { panic!("{outcome:?}") }; @@ -1321,6 +1372,19 @@ async fn a_province_under_its_new_name_finds_its_town_and_mid_forecast() { } let daily = body["midData"]["dailyData"].as_array().unwrap(); assert_eq!(daily.last().unwrap()["date"], json!("20261004")); + // The held request then stores its list. + zones.release(); + for _ in 0..100 { + if stored_zone_lists(&peer).await == 1 { + break; + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + assert_eq!( + stored_zone_lists(&peer).await, + 1, + "{region} {city}: the list" + ); } }