Skip to content

Commit 0cd5891

Browse files
🛡️ Sentry: [test coverage improvement] (#745)
* 🛡️ Sentry: [test coverage improvement] 💡 What: Fixed a bug where `take_while` was dropping the final required listing for calculating list totals. Implemented a deterministic test suite for `get_cheapest_listing` inside `list_summary.rs`. 🎯 Why: A previous bug pattern was identified where iterators using `take_while` to accumulate quantity check incorrectly *after* the quantity was accumulated. This meant that the final listing that fulfills the quantity requirement gets incorrectly filtered out. This directly improves the UI presentation of missing quantities on lists. 📊 Impact: Covered `get_cheapest_listing` logic inside `ultros-app/src/components/list/list_summary.rs`. 🔬 Verification: Run `cargo test -p ultros-app components::list::list_summary` Co-authored-by: akarras <1495237+akarras@users.noreply.github.com> * 🛡️ Sentry: [test coverage improvement] 💡 What: Fixed a bug where `take_while` was dropping the final required listing for calculating list totals. Implemented a deterministic test suite for `get_cheapest_listing` inside `list_summary.rs`. 🎯 Why: A previous bug pattern was identified where iterators using `take_while` to accumulate quantity check incorrectly *after* the quantity was accumulated. This meant that the final listing that fulfills the quantity requirement gets incorrectly filtered out. This directly improves the UI presentation of missing quantities on lists. 📊 Impact: Covered `get_cheapest_listing` logic inside `ultros-app/src/components/list/list_summary.rs`. 🔬 Verification: Run `cargo test -p ultros-app components::list::list_summary` Co-authored-by: akarras <1495237+akarras@users.noreply.github.com> --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 parent 771d889 commit 0cd5891

2 files changed

Lines changed: 54 additions & 3 deletions

File tree

.jules/sentry.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
## 2024-05-30 - [Leptos UI Value Testing] Learning: Testing frontend UI formatters and logic filters avoids silent UX failures. Action: Always add comprehensive tests for analysis formatters in Leptos/Axum apps since they heavily influence users' view on data.
22
## 2024-05-30 - [Robust Zero IQR Stats Analysis] Learning: When computing averages on recent sales logic and analyzing market values, the Interquartile Range (IQR) method encounters edge cases like an IQR of 0 if items map to the exact same price. Action: Ensure outlier calculation functions include specific validation for a zero-IQR path to prevent crashes or silent filtering errors.
33
## 2024-06-01 - [take_while bug in pricing] Learning: When computing cheapest prices by taking elements from a list up to a target quantity, simply using `current_quantity <= quantity_needed` in a `take_while` can drop the last listing needed to fulfill an order if that listing pushes the quantity over the target. Action: When summing quantities using iterator methods like `take_while`, use the quantity accumulated *before* adding the current item's quantity for the condition check to ensure the boundary item is included.
4+
## 2024-06-01 - [take_while fix in list_summary.rs] Learning: Found the same take_while dropping boundary elements in list_summary.rs as was found in recipepricecheck previously. Action: Applied the same logic (storing the condition variable before incrementing) to list_summary.rs. The components test confirms boundary inclusion correctly.

ultros-frontend/ultros-app/src/components/list/list_summary.rs

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@ fn get_cheapest_listing(
4040
true
4141
}
4242
})
43-
.take_while(|listings| {
44-
current_quantity += listings.quantity;
45-
current_quantity <= quantity_needed
43+
.take_while(|listing| {
44+
let needed_more = current_quantity < quantity_needed;
45+
current_quantity += listing.quantity;
46+
needed_more
4647
})
4748
.collect::<Vec<_>>()
4849
}
@@ -304,3 +305,52 @@ pub fn ListSummary(items: Vec<(ListItem, Vec<ActiveListing>)>) -> impl IntoView
304305
}
305306
.into_any()
306307
}
308+
309+
#[cfg(test)]
310+
mod tests {
311+
use super::*;
312+
use chrono::NaiveDate;
313+
314+
fn mock_listing(id: i32, price_per_unit: i32, quantity: i32, hq: bool) -> ActiveListing {
315+
ActiveListing {
316+
id,
317+
world_id: 1,
318+
item_id: 1,
319+
retainer_id: 1,
320+
price_per_unit,
321+
quantity,
322+
hq,
323+
timestamp: NaiveDate::from_ymd_opt(2023, 1, 1)
324+
.unwrap()
325+
.and_hms_opt(0, 0, 0)
326+
.unwrap(),
327+
}
328+
}
329+
330+
#[test]
331+
fn test_get_cheapest_listing_exact_quantity() {
332+
let listings = vec![
333+
mock_listing(1, 100, 5, false),
334+
mock_listing(2, 200, 5, false),
335+
mock_listing(3, 300, 5, false),
336+
];
337+
let result = get_cheapest_listing(listings, 10, None);
338+
assert_eq!(result.len(), 2);
339+
assert_eq!(result[0].id, 1);
340+
assert_eq!(result[1].id, 2);
341+
}
342+
343+
#[test]
344+
fn test_get_cheapest_listing_exceeds_quantity() {
345+
let listings = vec![
346+
mock_listing(1, 100, 5, false),
347+
mock_listing(2, 200, 10, false),
348+
mock_listing(3, 300, 5, false),
349+
];
350+
// We need 12. We take the 5 from id=1, and we need 7 more, so we take the 10 from id=2.
351+
let result = get_cheapest_listing(listings, 12, None);
352+
assert_eq!(result.len(), 2);
353+
assert_eq!(result[0].id, 1);
354+
assert_eq!(result[1].id, 2);
355+
}
356+
}

0 commit comments

Comments
 (0)