diff --git a/FuelBoard/ContentView.swift b/FuelBoard/ContentView.swift index ef723e7..db4f627 100644 --- a/FuelBoard/ContentView.swift +++ b/FuelBoard/ContentView.swift @@ -296,7 +296,7 @@ struct ContentView: View { /// that every station the app lists can carry a saving. private func savingsJourneyMiles(for station: FuelStation, location: Coordinate) -> FuelSavingsJourney? { if let roadMeters = FuelStore.roadJourneyMeters(for: station, userLat: location.lat, userLng: location.lng) { - return FuelSavingsJourney(miles: roadMeters / 1609.34, basis: .roadRoute) + return FuelSavingsJourney(miles: roadMeters / FuelSavingsCalculator.metresPerMile, basis: .roadRoute) } return FuelSavingsJourney( miles: savingsFallbackMiles(for: station, location: location), @@ -305,7 +305,7 @@ struct ContentView: View { } private func savingsFallbackMiles(for station: FuelStation, location: Coordinate) -> Double { - FuelStore.displayDistanceKM(station: station, userLat: location.lat, userLng: location.lng) * 0.621371 * 2 + FuelStore.displayDistanceKM(station: station, userLat: location.lat, userLng: location.lng) / FuelSavingsCalculator.kilometresPerMile * 2 } private func sortedForWorthTheDrive(_ pool: [FuelStation]) -> [FuelStation] { @@ -424,7 +424,12 @@ struct ContentView: View { StationDetailView( station: station, location: location, - distanceUnit: distanceUnit + distanceUnit: distanceUnit, + fuelSavings: savingsLookup(baseline: nearestSavingsBaseline)(station), + fuelSavingsAmount: sessionFuelSavingsAmount, + savingsBaselineStationName: nearestSavingsBaseline?.name, + isFavourite: favouriteIDs.contains(station.id), + onToggleFavourite: { toggleFavourite(station, fuel: selectedFuel) } ) } .onReceive(NotificationCenter.default.publisher(for: .fuelBoardShowStationDetail)) { note in @@ -525,6 +530,13 @@ struct ContentView: View { fuelSavingsSettings = forced sortMode = .worthTheDrive } + // QA temp: `FB_DETAIL_DEBUG=1` (DEBUG builds only) forces + // "Tap station = More info". The stored default is open-Apple-Maps, + // so on a simulator a tap bounces out to Maps with no way back and + // the Worth the Drive detail section can't be checked at all. + if ProcessInfo.processInfo.environment["FB_DETAIL_DEBUG"] == "1" { + stationTapAction = .showDetails + } // `-forceOfflineDump` / `-forceConnectionProblem` simulate the two // failure legs for the screenshot harness. The auto-refresh below // is skipped so the banner stays up (a live fetch would clear it). @@ -1242,6 +1254,10 @@ struct StationRow: View { var topBadgeText: String = "TOP" var tapAction: StationTapAction = .openMap var fuelSavings: FuelSavingsAvailability? + /// The session fill-up figure (the one the adjustment control edits). The + /// compact banner does not print it today, but a row that carried a hard + /// zero would print "for £0" the moment it did — pass the real figure. + var fuelSavingsAmount: FuelSavingsFillAmount = FuelSavingsSettings.defaultPreferredAmount var onToggleFavourite: () -> Void = {} var onShowDetails: (FuelStation) -> Void = { _ in } @@ -1263,6 +1279,13 @@ struct StationRow: View { return String(format: "%+.1fp", delta) } + /// True when this row IS the cheapest in range — its delta reads "best", + /// so labelling it "vs cheapest" would be nonsense. + private var isCheapestPrice: Bool { + guard let price = station.prices[fuel], let baselinePrice else { return false } + return abs(price - baselinePrice) <= 0.05 + } + private func handleTap() { switch tapAction { case .openMap: @@ -1279,6 +1302,7 @@ struct StationRow: View { } var body: some View { + VStack(alignment: .leading, spacing: 6) { HStack(spacing: 12) { // LEFT — round brand logo (or generic fuel pump fallback) Group { @@ -1306,18 +1330,6 @@ struct StationRow: View { .font(.headline) .lineLimit(1) .truncationMode(.tail) - if isTopResult { - // The badge always marks the CHEAPEST PUMP PRICE (never - // "best saving"), so in Worth the Drive order — where - // the top row is the best net saving — it may sit lower - // down the list, and says so. - Text(topBadgeText) - .font(.caption2.bold()) - .padding(.horizontal, 5) - .padding(.vertical, 1) - .background(Capsule().fill(.blue.opacity(0.15))) - .foregroundStyle(.blue) - } ForEach(StationNotice.notices(for: station)) { notice in Image(systemName: notice.symbolName) .font(.caption2.weight(.semibold)) @@ -1336,14 +1348,27 @@ struct StationRow: View { .foregroundStyle(.secondary) .monospacedDigit() } - if fuelSavings != nil { - FuelSavingsSummaryView(availability: fuelSavings, amount: .init(value: 0, unit: .pounds), baselineStationName: nil, compact: true) - } } .frame(maxWidth: .infinity, alignment: .leading) // RIGHT — price in £ (no p) + difference vs cheapest VStack(alignment: .trailing, spacing: 2) { + if isTopResult { + // The badge always marks the CHEAPEST PUMP PRICE (never + // "best saving"), so in Worth the Drive order — where the + // top row is the best net saving — it may sit lower down + // the list, and says so. It sits ABOVE the price it + // annotates rather than beside the name, so it reads as + // part of the price and the name keeps its width. + Text(topBadgeText) + .font(.caption2.bold()) + .padding(.horizontal, 5) + .padding(.vertical, 1) + .background(Capsule().fill(.blue.opacity(0.15))) + .foregroundStyle(.blue) + .lineLimit(1) + .fixedSize() + } if let price = station.prices[fuel] { HStack(spacing: 5) { Circle() @@ -1355,6 +1380,16 @@ struct StationRow: View { Text(deltaText) .font(.caption2.bold().monospaced()) .foregroundStyle(ragColor) + if !isCheapestPrice { + // "+8.0p" on its own never said what it measured + // against; the delta is against the cheapest price + // in range in EVERY sort mode. + Text("vs cheapest") + .font(.caption2) + .foregroundStyle(.secondary) + .lineLimit(1) + .minimumScaleFactor(0.8) + } } } } @@ -1367,6 +1402,20 @@ struct StationRow: View { .foregroundStyle(isFavourite ? .yellow : .secondary) } .buttonStyle(.borderless) + } + if fuelSavings != nil { + // Full-width verdict banner under the row. Inline in the middle + // column it truncated ("Save £2.75 after tra…") because the + // price block squeezes that column — and the verdict is the + // point of this sort mode, so it gets its own line. + FuelSavingsSummaryView( + availability: fuelSavings, + amount: fuelSavingsAmount, + baselineStationName: nil, + compact: true, + distanceUnit: distanceUnit + ) + } } .contentShape(Rectangle()) .onTapGesture(perform: handleTap) diff --git a/FuelBoard/FavouritesView.swift b/FuelBoard/FavouritesView.swift index b4ae646..252d3a1 100644 --- a/FuelBoard/FavouritesView.swift +++ b/FuelBoard/FavouritesView.swift @@ -167,6 +167,7 @@ struct FavouritesView: View { topBadgeText: "CHEAPEST", tapAction: stationTapAction, fuelSavings: savingsForStation(station), + fuelSavingsAmount: fuelSavingsAmount, onToggleFavourite: { onToggleFavourite(station, activeFuel) }, onShowDetails: { detailStation = $0 } ) @@ -217,7 +218,11 @@ struct FavouritesView: View { distanceUnit: distanceUnit, fuelSavings: savingsForStation(station), fuelSavingsAmount: fuelSavingsAmount, - savingsBaselineStationName: savingsBaselineStationName + savingsBaselineStationName: savingsBaselineStationName, + // Every station listed here IS a favourite for the tab's + // fuel, so the star starts filled; tapping it unstars. + isFavourite: true, + onToggleFavourite: { onToggleFavourite(station, activeFuel) } ) } .onAppear { diff --git a/FuelBoard/FuelSavingsViews.swift b/FuelBoard/FuelSavingsViews.swift index df0f392..a6713e2 100644 --- a/FuelBoard/FuelSavingsViews.swift +++ b/FuelBoard/FuelSavingsViews.swift @@ -5,6 +5,10 @@ struct FuelSavingsAdjustmentView: View { let referencePricePencePerLitre: Double? @State private var textValue: String = "" + /// Focus for the numeric field — also lets the DEBUG QA hook below raise the + /// keyboard on launch so the Done bar can be screenshotted (a script cannot + /// tap the simulator). + @FocusState private var isAmountFieldFocused: Bool private var range: ClosedRange { amount.unit == .pounds ? 10...100 : 10...80 @@ -27,12 +31,25 @@ struct FuelSavingsAdjustmentView: View { .multilineTextAlignment(.trailing) .frame(width: 72) .textFieldStyle(.roundedBorder) + .focused($isAmountFieldFocused) + .accessibilityIdentifier("fillAmountField") } Text("Changes the estimate for this session only — your Settings default stays unchanged.") .font(.caption2) .foregroundStyle(.secondary) } - .onAppear { syncText() } + .onAppear { + syncText() +#if DEBUG + // QA hook: raise the keyboard without a tap so the Done bar can be + // screenshotted on a simulator (synthetic taps do not reach it). + if ProcessInfo.processInfo.arguments.contains("-focusFillAmount") { + DispatchQueue.main.asyncAfter(deadline: .now() + 1.0) { + isAmountFieldFocused = true + } + } +#endif + } .onChange(of: amount.value) { _, _ in syncText() } .onChange(of: amount.unit) { _, _ in syncText() } } @@ -83,16 +100,53 @@ struct FuelSavingsSummaryView: View { let amount: FuelSavingsFillAmount let baselineStationName: String? var compact: Bool = false + /// Sizes the "more driving than closest" caption in the user's own unit (the + /// stored figure is miles) — used by both the row banner and the detail sheet. + var distanceUnit: DistanceUnit = .miles var body: some View { switch availability { case .available(let result): - VStack(alignment: .leading, spacing: compact ? 2 : 8) { - Label(result.status.displayName, systemImage: symbol(for: result.status)) - .font(compact ? .caption.weight(.semibold) : .headline) - Text(result.headline(compact: compact)) - .font(compact ? .caption2.bold().monospacedDigit() : .subheadline.bold().monospacedDigit()) - if !compact { + if compact { + // FULL-WIDTH banner under the row. Inline inside the middle + // column the capsule truncated ("Save £2.75 after tra…") because + // the column is squeezed by the price block — the verdict is the + // point of this sort mode, so it gets its own line with room. + // Colour carries the verdict: green save, red worse off, grey + // break even, blue baseline. lineLimit + minimumScaleFactor keep + // it to one line, so rows stay even. + VStack(alignment: .leading, spacing: 2) { + capsule(for: result, compact: true) + if let extraTravel = FuelSavingsFormat.extraTravelLabel( + extraDistanceMiles: result.extraDistanceMiles, + unit: distanceUnit + ) { + Text(extraTravel) + .font(.caption2) + .foregroundStyle(.secondary) + .lineLimit(1) + .minimumScaleFactor(0.85) + } + } + // Aligns the banner with the row's text column (46 pt logo + 12 + // pt spacing) so it reads as part of the row, not the cell. + .padding(.leading, 58) + } else { + // Detail sheet: the SAME verdict capsule and the SAME caption as + // the listing rows (one implementation, so wording and colour + // cannot drift), followed by the breakdown only this view has + // room for. The old shape led with a status word + headline that + // matched neither the listing nor each other. + VStack(alignment: .leading, spacing: 10) { + capsule(for: result, compact: false) + if let extraTravel = FuelSavingsFormat.extraTravelLabel( + extraDistanceMiles: result.extraDistanceMiles, + unit: distanceUnit + ) { + Text(extraTravel) + .font(.subheadline) + .foregroundStyle(.secondary) + } if result.status == .baseline { Text("Your closest station with a price for this fuel — every other saving is measured against it.") .font(.caption) @@ -101,7 +155,6 @@ struct FuelSavingsSummaryView: View { Text("Compared with \(baselineStationName ?? "your nearest station") for \(amountText).").font(.caption).foregroundStyle(.secondary) LabeledContent("Fuel-price saving", value: FuelSavingsFormat.money(result.grossSavingGBP)) LabeledContent("Extra driving cost", value: FuelSavingsFormat.money(result.extraDrivingCostGBP)) - LabeledContent("Extra distance", value: String(format: "%.1f miles", result.extraDistanceMiles)) if result.distanceBasis.isEstimate { Text("Estimated from straight-line distance — refined once the route is measured.") .font(.caption2) @@ -129,6 +182,35 @@ struct FuelSavingsSummaryView: View { } } + /// The colour-coded verdict capsule — ONE implementation for the listing rows + /// and the detail sheet, so the verdict reads identically in both; the sheet + /// just gets the larger font and padding. + private func capsule(for result: FuelSavingsResult, compact: Bool) -> some View { + Label { + Text(result.capsuleHeadline()) + .lineLimit(1) + .minimumScaleFactor(0.8) + } icon: { + Image(systemName: symbol(for: result.status)) + } + .font(compact ? .caption.weight(.semibold) : .headline) + .foregroundStyle(capsuleColor(for: result.status)) + .padding(.horizontal, compact ? 8 : 12) + .padding(.vertical, compact ? 4 : 7) + .background(Capsule().fill(capsuleColor(for: result.status).opacity(0.15))) + } + + /// Verdict colour: green save, red worse off, grey break even, blue + /// baseline — the same green/amber/red family the price dot already uses. + private func capsuleColor(for status: FuelSavingsStatus) -> Color { + switch status { + case .worthTheDrive: return .green + case .notWorthTheDrive: return .red + case .breakEven: return .secondary + case .baseline: return .blue + } + } + private func symbol(for status: FuelSavingsStatus) -> String { switch status { case .worthTheDrive: return "checkmark.circle.fill" diff --git a/FuelBoard/KeyboardSupport.swift b/FuelBoard/KeyboardSupport.swift index a89f2f5..c017ed3 100644 --- a/FuelBoard/KeyboardSupport.swift +++ b/FuelBoard/KeyboardSupport.swift @@ -1,5 +1,6 @@ import SwiftUI import UIKit +import Combine extension View { /// Puts a Done button above the keyboard. @@ -11,31 +12,68 @@ extension View { /// `TextField(value:format:)` only commits the typed value when editing /// ends — so the number never stuck either. /// - /// Apply ONCE per screen, to the list or form containing the fields: the - /// keyboard toolbar belongs to the focused field, so a single attachment - /// covers every field inside it. Attaching it to each field would show - /// several Done buttons at once. + /// WHY IT IS DRAWN HERE RATHER THAN AS `.toolbar(placement: .keyboard)`: + /// the keyboard-toolbar form is device-dependent. The same build showed the + /// Done button on the iPhone 15 Pro and NOT on the iPhone SE (both iOS + /// 27.0.1, same install, same binary), leaving the SE with no way to + /// dismiss or commit a numeric field. Drawing the bar in the screen's own + /// bottom safe area renders identically on every device, so the accessibility + /// of the numeric fields no longer depends on the accessory view appearing. + /// + /// Apply ONCE per screen, to the list or form containing the fields: any + /// keyboard on that screen shows the bar, so attaching it to each field + /// would stack several Done buttons. /// /// - Parameter action: clears the screen's focus state. Prefer this where a /// `@FocusState` exists; the default resigns the current first responder, - /// which works for fields inside subviews that have no focus binding. + /// which works for fields inside subviews that have no focus binding (the + /// fill-up box in the station list is one). func keyboardDoneButton(action: (() -> Void)? = nil) -> some View { - toolbar { - ToolbarItemGroup(placement: .keyboard) { - Spacer() - Button("Done") { - if let action { - action() - } else { - UIApplication.shared.sendAction( - #selector(UIResponder.resignFirstResponder), - to: nil, - from: nil, - for: nil - ) - } - } - } - } + modifier(KeyboardDoneBar(action: action)) + } +} + +/// Shows a Done bar while a keyboard is up on this screen. +/// +/// Visibility comes from the keyboard notifications rather than a `@FocusState` +/// so the bar also works for fields inside subviews that have no focus binding. +private struct KeyboardDoneBar: ViewModifier { + let action: (() -> Void)? + + @State private var isKeyboardVisible = false + + func body(content: Content) -> some View { + content + .safeAreaInset(edge: .bottom, spacing: 0) { + if isKeyboardVisible { + HStack { + Spacer() + Button("Done") { + if let action { + action() + } else { + UIApplication.shared.sendAction( + #selector(UIResponder.resignFirstResponder), + to: nil, + from: nil, + for: nil + ) + } + isKeyboardVisible = false + } + .font(.body.weight(.semibold)) + } + .padding(.horizontal, 20) + .padding(.vertical, 10) + .frame(maxWidth: .infinity) + .background(.bar) + } + } + .onReceive(NotificationCenter.default.publisher(for: UIResponder.keyboardWillShowNotification)) { _ in + isKeyboardVisible = true + } + .onReceive(NotificationCenter.default.publisher(for: UIResponder.keyboardWillHideNotification)) { _ in + isKeyboardVisible = false + } } } diff --git a/FuelBoard/StationDetailView.swift b/FuelBoard/StationDetailView.swift index 232766c..9aefaf4 100644 --- a/FuelBoard/StationDetailView.swift +++ b/FuelBoard/StationDetailView.swift @@ -13,6 +13,12 @@ struct StationDetailView: View { var fuelSavings: FuelSavingsAvailability? = nil var fuelSavingsAmount: FuelSavingsFillAmount = FuelSavingsSettings.defaultPreferredAmount var savingsBaselineStationName: String? = nil + /// Whether this station is starred for the fuel currently being viewed, and + /// the toggle that flips it. Passed from the list that opened the sheet, so + /// the star here and the star on the row are the same fuel-scoped + /// favourite and cannot drift apart. + var isFavourite: Bool = false + var onToggleFavourite: (() -> Void)? = nil @Environment(\.dismiss) private var dismiss @State private var showingReportIssueConfirmation = false @@ -61,7 +67,8 @@ struct StationDetailView: View { FuelSavingsSummaryView( availability: fuelSavings, amount: fuelSavingsAmount, - baselineStationName: savingsBaselineStationName + baselineStationName: savingsBaselineStationName, + distanceUnit: distanceUnit ) } header: { Text("Worth the drive?") @@ -96,6 +103,20 @@ struct StationDetailView: View { ToolbarItem(placement: .cancellationAction) { Button("Done") { dismiss() } } + ToolbarItem(placement: .confirmationAction) { + Button { + onToggleFavourite?() + } label: { + Image(systemName: isFavourite ? "star.fill" : "star") + } + .tint(isFavourite ? Color.yellow : nil) + .disabled(onToggleFavourite == nil) + .accessibilityLabel( + isFavourite + ? String(localized: "Remove from favourites", comment: "Unstar a station from the More info sheet") + : String(localized: "Add to favourites", comment: "Star a station from the More info sheet") + ) + } } } } diff --git a/FuelBoard/StationsView.swift b/FuelBoard/StationsView.swift index 7cebb0b..feb29a0 100644 --- a/FuelBoard/StationsView.swift +++ b/FuelBoard/StationsView.swift @@ -155,6 +155,7 @@ struct StationsView: View { topBadgeText: sortMode == .worthTheDrive ? "CHEAPEST" : "TOP", tapAction: stationTapAction, fuelSavings: savingsForStation(station), + fuelSavingsAmount: fuelSavingsAmount, onToggleFavourite: { onToggleFavourite(station, selectedFuel) }, onShowDetails: { detailStation = $0 } ) @@ -215,6 +216,18 @@ struct StationsView: View { if ProcessInfo.processInfo.arguments.contains("-showKeySheet") { showKey = true } +#endif + } + .task(id: stations.count) { +#if DEBUG + // QA hook: `-showDetail` opens the More info sheet for the first + // listed station, so the sheet can be screenshotted without a + // tap. Waits for the list to arrive first. + guard ProcessInfo.processInfo.arguments.contains("-showDetail"), + detailStation == nil, + let first = stations.first else { return } + try? await Task.sleep(nanoseconds: 2_000_000_000) + detailStation = first #endif } .toolbar { @@ -302,7 +315,9 @@ struct StationsView: View { distanceUnit: distanceUnit, fuelSavings: savingsForStation(station), fuelSavingsAmount: fuelSavingsAmount, - savingsBaselineStationName: savingsBaselineStationName + savingsBaselineStationName: savingsBaselineStationName, + isFavourite: favouriteIDs.contains(station.id), + onToggleFavourite: { onToggleFavourite(station, selectedFuel) } ) } } diff --git a/FuelBoard/en.lproj/Localizable.strings b/FuelBoard/en.lproj/Localizable.strings index a11dff5..8a0328e 100644 --- a/FuelBoard/en.lproj/Localizable.strings +++ b/FuelBoard/en.lproj/Localizable.strings @@ -31,6 +31,9 @@ "More than 4p over the cheapest" = "More than 4p over the cheapest"; "Top result for the current sort" = "Top result for the current sort"; "Star a station to add it to Favourites" = "Star a station to add it to Favourites"; +/* Star button in the More info sheet heading (accessibility labels) */ +"Add to favourites" = "Add to favourites"; +"Remove from favourites" = "Remove from favourites"; "Colours match each station's rating on the list" = "Colours match each station's rating on the list"; /* Favourites tab */ diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift new file mode 100644 index 0000000..62ac058 --- /dev/null +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsCalculationAuditTests.swift @@ -0,0 +1,419 @@ +import XCTest +#if SWIFT_PACKAGE +@testable import FuelBoardShared +#endif + +/// Worth the Drive calculation audit. +/// +/// Written against the audit brief (30 MPG imperial, £60 fill-up, baseline +/// Rontec Shelf at 171.9p, 0.4 miles). The brief's three observed results are +/// reproduced here exactly, so any future change to the formula breaks a test +/// instead of silently shifting the numbers on screen. +/// +/// Deterministic journey distances only — never live MKDirections responses. +final class FuelSavingsCalculationAuditTests: XCTestCase { + private func station(_ id: String, price: Double?) -> FuelStation { + FuelStation( + id: id, + name: id, + brand: "X", + address: "", + postcode: "", + lat: 51.5, + lng: -0.1, + prices: price.map { [FuelType.e10: $0] } ?? [:], + priceUpdated: nil + ) + } + + /// Journeys are OUT AND BACK (the app's convention): the brief's baseline is + /// 0.4 miles away, so its journey is 0.8 miles, and "extra driving" of N + /// miles means a candidate journey of 0.8 + N. + private func journey(_ miles: Double, basis: FuelSavingsDistanceBasis = .roadRoute) -> FuelSavingsJourney { + FuelSavingsJourney(miles: miles, basis: basis) + } + + private let baselineJourneyMiles = 0.8 + + private func briefResult(candidatePrice: Double, extraMiles: Double) throws -> FuelSavingsResult { + try FuelSavingsCalculator.calculate( + mpg: 30, + amount: FuelSavingsFillAmount(value: 60, unit: .pounds), + baselinePricePencePerLitre: 171.9, + candidatePricePencePerLitre: candidatePrice, + baselineJourneyMiles: baselineJourneyMiles, + candidateJourneyMiles: baselineJourneyMiles + extraMiles + ) + } + + // MARK: - 1. Unit conversion + + func testImperialGallonIsUsedNotTheUSGallon() { + XCTAssertEqual(FuelSavingsCalculator.imperialGallonLitres, 4.54609) + XCTAssertNotEqual(FuelSavingsCalculator.imperialGallonLitres, 3.785411784, + "a US gallon must never be used: it would understate every travel cost") + + // 30 miles at 30 MPG burns exactly one imperial gallon. + XCTAssertEqual(FuelSavingsCalculator.fuelUsedLitres(distanceMiles: 30, mpg: 30), 4.54609, accuracy: 1e-12) + // 100 miles at 50 MPG. + XCTAssertEqual(FuelSavingsCalculator.fuelUsedLitres(distanceMiles: 100, mpg: 50), 9.09218, accuracy: 1e-12) + // Zero distance burns nothing, and the relationship is linear in distance. + XCTAssertEqual(FuelSavingsCalculator.fuelUsedLitres(distanceMiles: 0, mpg: 30), 0, accuracy: 1e-12) + XCTAssertEqual(FuelSavingsCalculator.fuelUsedLitres(distanceMiles: 60, mpg: 30), + 2 * FuelSavingsCalculator.fuelUsedLitres(distanceMiles: 30, mpg: 30), + accuracy: 1e-12) + } + + func testMileAndKilometreFactorsAreTheExactOnes() { + XCTAssertEqual(FuelSavingsCalculator.kilometresPerMile, 1.609344) + XCTAssertEqual(FuelSavingsCalculator.metresPerMile, 1609.344) + XCTAssertEqual(FuelSavingsCalculator.metresPerMile, + FuelSavingsCalculator.kilometresPerMile * 1000, + accuracy: 1e-9, + "the metre and kilometre factors must describe the same mile") + } + + func testDistanceLabelUsesTheSameConversionAsTheModel() { + // One mile, shown in kilometres, goes through the single factor. + XCTAssertEqual( + FuelSavingsFormat.distance(1, unit: .kilometers), + DistanceUnit.kilometers.format(FuelSavingsCalculator.kilometresPerMile) + ) + XCTAssertEqual(FuelSavingsFormat.distance(1, unit: .miles), DistanceUnit.miles.format(1.609344)) + } + + // MARK: - 2/4. The three stations from the brief + + func testBriefCaseTescoBradfordIsMarginallyWorseOff() throws { + let result = try briefResult(candidatePrice: 170.9, extraMiles: 2.0) + + XCTAssertEqual(result.fillLitres, 34.904014, accuracy: 1e-6) + XCTAssertEqual(result.grossSavingGBP, 0.349040, accuracy: 1e-6, "1p/litre cheaper on a 34.90 L fill") + XCTAssertEqual(result.extraDistanceMiles, 2.0, accuracy: 1e-9) + XCTAssertEqual(result.extraDrivingCostGBP, 0.517951, accuracy: 1e-6) + XCTAssertEqual(result.netSavingGBP, -0.168911, accuracy: 1e-6) + XCTAssertEqual(result.status, .notWorthTheDrive) + XCTAssertEqual(result.headline(), "£0.17 worse off after travel") + } + + /// The brief recorded "£0.18 worse off" for this row against a displayed + /// "2.0 miles" of extra driving; the brief also warns that the displayed + /// mileage is rounded. This pins that explanation instead of treating it as + /// a calculation bug: at an extra distance the caption still shows as + /// "2.0 mi" (2.00-2.05), the headline reads £0.17-£0.18, and the difference + /// is entirely the rounding of the distance, not of the money. + func testTheReportedEighteenPenceIsRoundedExtraDistanceNotARoundingBug() throws { + XCTAssertEqual(try briefResult(candidatePrice: 170.9, extraMiles: 2.00).headline(), + "£0.17 worse off after travel") + XCTAssertEqual(try briefResult(candidatePrice: 170.9, extraMiles: 2.05).headline(), + "£0.18 worse off after travel") + + // 2.05 mi shows as "2.0 mi" at one decimal place — the same caption. + XCTAssertEqual(FuelSavingsFormat.extraTravelLabel(extraDistanceMiles: 2.05, unit: .miles), + "2.0 mi more driving than closest") + XCTAssertEqual(FuelSavingsFormat.extraTravelLabel(extraDistanceMiles: 2.00, unit: .miles), + "2.0 mi more driving than closest") + } + + func testBriefCaseMFGHareAndHoundsIsWorseOff() throws { + let result = try briefResult(candidatePrice: 172.9, extraMiles: 2.8) + + XCTAssertEqual(result.fillLitres, 34.904014, accuracy: 1e-6) + XCTAssertEqual(result.grossSavingGBP, -0.349040, accuracy: 1e-6, "1p/litre dearer costs 34.90 L × 1p") + XCTAssertEqual(result.extraDistanceMiles, 2.8, accuracy: 1e-9) + XCTAssertEqual(result.extraDrivingCostGBP, 0.733618, accuracy: 1e-6) + XCTAssertEqual(result.netSavingGBP, -1.082658, accuracy: 1e-6) + XCTAssertEqual(result.status, .notWorthTheDrive) + XCTAssertEqual(result.headline(), "£1.08 worse off after travel") + } + + func testBriefCasePGGNapoleonSamePumpPriceIsTravelCostOnly() throws { + let result = try briefResult(candidatePrice: 171.9, extraMiles: 2.8) + + XCTAssertEqual(result.grossSavingGBP, 0, accuracy: 1e-9, "same pump price saves nothing at the pump") + XCTAssertEqual(result.extraDrivingCostGBP, 0.729375, accuracy: 1e-6) + XCTAssertEqual(result.netSavingGBP, -0.729375, accuracy: 1e-6, + "with no price difference the whole result is the extra driving") + XCTAssertEqual(result.status, .notWorthTheDrive) + XCTAssertEqual(result.headline(), "£0.73 worse off after travel") + } + + // MARK: - 5/6. Classification, tolerance and rounding + + func testSamePriceAndSameDistanceIsBreakEven() throws { + let result = try briefResult(candidatePrice: 171.9, extraMiles: 0) + XCTAssertEqual(result.grossSavingGBP, 0, accuracy: 1e-9) + XCTAssertEqual(result.extraDrivingCostGBP, 0, accuracy: 1e-9) + XCTAssertEqual(result.netSavingGBP, 0, accuracy: 1e-9) + XCTAssertEqual(result.status, .breakEven) + XCTAssertEqual(result.headline(), "Break even") + } + + func testSmallSavingInsideTheToleranceIsBreakEvenNotPennies() throws { + // 171.9p vs 170.9p over a 1.23 mi delta: net +£0.0305. + let result = try briefResult(candidatePrice: 170.9, extraMiles: 1.23) + XCTAssertEqual(result.netSavingGBP, 0.030500, accuracy: 1e-6) + XCTAssertEqual(result.status, .breakEven, "±5p reads as break even rather than a meaningless £0.03") + XCTAssertEqual(result.headline(), "Break even") + } + + func testBreakEvenToleranceDecidesTheClassificationAtTheBoundary() throws { + let net = -0.168911 + // A tolerance wider than the loss → break even; narrower → worse off. + let lenient = try FuelSavingsCalculator.calculate( + mpg: 30, + amount: FuelSavingsFillAmount(value: 60, unit: .pounds), + baselinePricePencePerLitre: 171.9, + candidatePricePencePerLitre: 170.9, + baselineJourneyMiles: baselineJourneyMiles, + candidateJourneyMiles: baselineJourneyMiles + 2.0, + breakEvenToleranceGBP: 0.17 + ) + XCTAssertEqual(lenient.netSavingGBP, net, accuracy: 1e-6) + XCTAssertEqual(lenient.status, .breakEven) + + let strict = try FuelSavingsCalculator.calculate( + mpg: 30, + amount: FuelSavingsFillAmount(value: 60, unit: .pounds), + baselinePricePencePerLitre: 171.9, + candidatePricePencePerLitre: 170.9, + baselineJourneyMiles: baselineJourneyMiles, + candidateJourneyMiles: baselineJourneyMiles + 2.0, + breakEvenToleranceGBP: 0.16 + ) + XCTAssertEqual(strict.status, .notWorthTheDrive) + } + + func testMoneyIsRoundedOnlyForDisplay() throws { + // Full precision retained internally… + let result = try briefResult(candidatePrice: 172.9, extraMiles: 2.8) + XCTAssertEqual(result.netSavingGBP, -1.082658, accuracy: 1e-6) + XCTAssertNotEqual(result.netSavingGBP, -1.08, "the stored value must not be pre-rounded") + // …and rounded to two places only when shown. + XCTAssertEqual(FuelSavingsFormat.money(1.082658), "£1.08") + XCTAssertEqual(FuelSavingsFormat.money(0.729375), "£0.73") + XCTAssertEqual(FuelSavingsFormat.money(0.1749), "£0.17") + XCTAssertEqual(FuelSavingsFormat.money(0.176), "£0.18") + XCTAssertEqual(FuelSavingsFormat.money(0), "£0.00") + } + + func testSortingUsesTheUnroundedNetSoRoundedTiesStillOrderCorrectly() { + // Two rows that both DISPLAY "£0.18 worse off" but differ in the raw + // figure: the sort must use the raw value, never the shown one. + func ranked(_ id: String, net: Double) -> FuelSavingsRankedStation { + FuelSavingsRankedStation( + stationID: id, + result: .available(FuelSavingsResult( + fillLitres: 34.9, + grossSavingGBP: 0, + extraDrivingCostGBP: abs(net), + extraDistanceMiles: 2.0, + netSavingGBP: net, + status: .notWorthTheDrive + )), + distanceMiles: 2.8, + pricePencePerLitre: 171.9 + ) + } + let worse = ranked("worse", net: -0.1751) + let lessWorse = ranked("less-worse", net: -0.1749) + XCTAssertEqual(FuelSavingsFormat.money(abs(worse.result.result!.netSavingGBP)), "£0.18") + XCTAssertEqual(FuelSavingsFormat.money(abs(lessWorse.result.result!.netSavingGBP)), "£0.17") + + let sorted = FuelSavingsCalculator.sortedForWorthTheDrive([worse, lessWorse]) + XCTAssertEqual(sorted.first?.stationID, "less-worse", + "the smaller loss must rank first even when the money rounds to the same figure") + } + + // MARK: - 3. Distances and travel cost + + func testCheaperFuelFurtherAwayCanStillBeWorthTheDrive() throws { + // 145.9p nearby vs 138.9p five miles further out, £40 fill, 45 MPG. + let result = try FuelSavingsCalculator.calculate( + mpg: 45, + amount: FuelSavingsFillAmount(value: 40, unit: .pounds), + baselinePricePencePerLitre: 145.9, + candidatePricePencePerLitre: 138.9, + baselineJourneyMiles: 2.0, + candidateJourneyMiles: 7.0 + ) + XCTAssertEqual(result.fillLitres, 27.416038, accuracy: 1e-6) + XCTAssertEqual(result.grossSavingGBP, 1.919123, accuracy: 1e-6) + XCTAssertEqual(result.extraDrivingCostGBP, 0.701613, accuracy: 1e-6) + XCTAssertEqual(result.netSavingGBP, 1.217509, accuracy: 1e-6) + XCTAssertEqual(result.status, .worthTheDrive) + XCTAssertEqual(result.headline(), "Save £1.22 after travel") + } + + func testNearerCandidateIsNeverChargedExtraTravel() throws { + // The baseline is the nearest priced station, so this should not happen + // in the app — but the clamp must hold if it ever does. + let result = try FuelSavingsCalculator.calculate( + mpg: 30, + amount: FuelSavingsFillAmount(value: 60, unit: .pounds), + baselinePricePencePerLitre: 171.9, + candidatePricePencePerLitre: 169.9, + baselineJourneyMiles: 2.0, + candidateJourneyMiles: 1.0 + ) + XCTAssertEqual(result.extraDistanceMiles, 0, "a shorter journey is never a negative extra distance") + XCTAssertEqual(result.extraDrivingCostGBP, 0, accuracy: 1e-12) + XCTAssertEqual(result.netSavingGBP, result.grossSavingGBP, accuracy: 1e-12) + } + + func testDifferentMPGValuesScaleOnlyTheTravelCost() throws { + let sixty = try FuelSavingsCalculator.calculate( + mpg: 60, + amount: FuelSavingsFillAmount(value: 60, unit: .pounds), + baselinePricePencePerLitre: 171.9, + candidatePricePencePerLitre: 172.9, + baselineJourneyMiles: 0.8, + candidateJourneyMiles: 3.6 + ) + XCTAssertEqual(sixty.grossSavingGBP, -0.349040, accuracy: 1e-6, "MPG must not touch the pump saving") + XCTAssertEqual(sixty.extraDrivingCostGBP, 0.366809, accuracy: 1e-6) + XCTAssertEqual(sixty.netSavingGBP, -0.715849, accuracy: 1e-6) + + let fortyFive = try FuelSavingsCalculator.calculate( + mpg: 45, + amount: FuelSavingsFillAmount(value: 60, unit: .pounds), + baselinePricePencePerLitre: 171.9, + candidatePricePencePerLitre: 172.9, + baselineJourneyMiles: 0.8, + candidateJourneyMiles: 3.6 + ) + XCTAssertEqual(fortyFive.extraDrivingCostGBP, 0.489078, accuracy: 1e-6) + XCTAssertEqual(fortyFive.netSavingGBP, -0.838119, accuracy: 1e-6) + } + + func testDifferentFillBudgetsScaleOnlyThePumpSaving() throws { + let forty = try FuelSavingsCalculator.calculate( + mpg: 30, + amount: FuelSavingsFillAmount(value: 40, unit: .pounds), + baselinePricePencePerLitre: 171.9, + candidatePricePencePerLitre: 172.9, + baselineJourneyMiles: 0.8, + candidateJourneyMiles: 3.6 + ) + XCTAssertEqual(forty.fillLitres, 23.269343, accuracy: 1e-6, "£40 buys 23.27 L at 171.9p") + XCTAssertEqual(forty.grossSavingGBP, -0.232693, accuracy: 1e-6) + XCTAssertEqual(forty.extraDrivingCostGBP, 0.733618, accuracy: 1e-6, "the journey cost is budget-independent") + XCTAssertEqual(forty.netSavingGBP, -0.966311, accuracy: 1e-6) + + let ninety = try FuelSavingsCalculator.calculate( + mpg: 30, + amount: FuelSavingsFillAmount(value: 90, unit: .pounds), + baselinePricePencePerLitre: 171.9, + candidatePricePencePerLitre: 172.9, + baselineJourneyMiles: 0.8, + candidateJourneyMiles: 3.6 + ) + XCTAssertEqual(ninety.fillLitres, 52.356021, accuracy: 1e-6) + XCTAssertEqual(ninety.grossSavingGBP, -0.523560, accuracy: 1e-6) + XCTAssertEqual(ninety.netSavingGBP, -1.257178, accuracy: 1e-6) + } + + func testFillUpInLitresIsFixedRegardlessOfPrice() throws { + // 40 L is 40 L: no baseline price is involved in the quantity. + let result = try FuelSavingsCalculator.calculate( + mpg: 30, + amount: FuelSavingsFillAmount(value: 40, unit: .litres), + baselinePricePencePerLitre: 171.9, + candidatePricePencePerLitre: 165.9, + baselineJourneyMiles: 0.8, + candidateJourneyMiles: 3.6 + ) + XCTAssertEqual(result.fillLitres, 40, accuracy: 1e-9) + XCTAssertEqual(result.grossSavingGBP, 40 * 0.06, accuracy: 1e-9) + } + + // MARK: - 7. Failures never invent a saving + + func testUnavailableRouteProducesNoSavingAtAll() { + let baseline = station("baseline", price: 171.9) + let candidate = station("candidate", price: 170.9) + let settings = FuelSavingsSettings( + enabled: true, + mpg: 30, + preferredAmount: FuelSavingsFillAmount(value: 60, unit: .pounds) + ) + + let noRoute = FuelSavingsEstimator.availability( + station: candidate, + fuel: .e10, + baseline: baseline, + settings: settings, + amount: FuelSavingsFillAmount(value: 60, unit: .pounds), + journey: nil, + baselineJourney: journey(0.8) + ) + XCTAssertEqual(noRoute, .unavailable("Distance unavailable")) + + let noBaselineRoute = FuelSavingsEstimator.availability( + station: candidate, + fuel: .e10, + baseline: baseline, + settings: settings, + amount: FuelSavingsFillAmount(value: 60, unit: .pounds), + journey: journey(2.8), + baselineJourney: nil + ) + XCTAssertEqual(noBaselineRoute, .unavailable("No comparison station")) + + // With both legs present it produces the saving — no silent zero. + let both = FuelSavingsEstimator.availability( + station: candidate, + fuel: .e10, + baseline: baseline, + settings: settings, + amount: FuelSavingsFillAmount(value: 60, unit: .pounds), + journey: journey(2.8), + baselineJourney: journey(0.8) + ) + XCTAssertEqual(both?.result?.netSavingGBP ?? 0, -0.168911, accuracy: 1e-6) + } + + func testMixedRouteBasisIsReportedAsAnEstimate() { + let baseline = station("baseline", price: 171.9) + let candidate = station("candidate", price: 170.9) + let settings = FuelSavingsSettings( + enabled: true, + mpg: 30, + preferredAmount: FuelSavingsFillAmount(value: 60, unit: .pounds) + ) + let mixed = FuelSavingsEstimator.availability( + station: candidate, + fuel: .e10, + baseline: baseline, + settings: settings, + amount: FuelSavingsFillAmount(value: 60, unit: .pounds), + journey: journey(2.8, basis: .straightLine), + baselineJourney: journey(0.8, basis: .roadRoute) + ) + XCTAssertEqual(mixed?.result?.distanceBasis, .straightLine, + "one unmeasured leg makes the whole estimate an estimate") + let headline = mixed?.result?.headline(compact: true) ?? "" + XCTAssertTrue(headline.hasSuffix(" (est.)"), "an estimate must be marked as one, got \(headline)") + } + + func testBaselineStationIsLabelledClosestStationNotBreakEven() { + let baseline = station("baseline", price: 171.9) + let settings = FuelSavingsSettings( + enabled: true, + mpg: 30, + preferredAmount: FuelSavingsFillAmount(value: 60, unit: .pounds) + ) + let result = FuelSavingsEstimator.availability( + station: baseline, + fuel: .e10, + baseline: baseline, + settings: settings, + amount: FuelSavingsFillAmount(value: 60, unit: .pounds), + journey: journey(0.8), + baselineJourney: journey(0.8) + )?.result + XCTAssertEqual(result?.status, .baseline) + XCTAssertEqual(result?.netSavingGBP, 0) + XCTAssertEqual(result?.headline(), "Closest station") + } +} diff --git a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift index 0bb3d49..8800c00 100644 --- a/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift +++ b/FuelBoardTests/Tests/FuelBoardSharedTests/FuelSavingsEstimateTests.swift @@ -374,4 +374,62 @@ final class FuelSavingsEstimateTests: XCTestCase { "net saving decides the order, not the pump price") XCTAssertEqual(ranked.last?.id, "cheap-but-far") } + + // MARK: Row copy + + /// The row's extra-driving caption must follow the DISPLAY unit: the stored + /// figure is always miles, and converting by the current unit instead would + /// leave a km user's figure unlabelled and 1.6× too large. + func testExtraTravelLabelFollowsTheDisplayUnit() { + XCTAssertEqual( + FuelSavingsFormat.extraTravelLabel(extraDistanceMiles: 19.9, unit: .miles), + "19.9 mi more driving than closest" + ) + XCTAssertEqual( + FuelSavingsFormat.extraTravelLabel(extraDistanceMiles: 19.9, unit: .kilometers), + "32.0 km more driving than closest" + ) + XCTAssertNil(FuelSavingsFormat.extraTravelLabel(extraDistanceMiles: 0, unit: .miles), + "the baseline row has no extra travel to report") + // The same conversion backs the detail sheet's extra-distance figure, + // which used to be hardcoded miles ("%.1f miles") for km users too. + XCTAssertEqual(FuelSavingsFormat.distance(19.9, unit: .miles), "19.9 mi") + XCTAssertEqual(FuelSavingsFormat.distance(19.9, unit: .kilometers), "32.0 km") + } + + /// The baseline capsule has to say what it IS: it previously repeated the + /// status word above it ("Closest station" twice) and never explained that + /// the savings are measured against it. + func testCapsuleHeadlineMarksTheBaselineAndKeepsTheEstimateMarker() { + let baseline = FuelSavingsResult( + fillLitres: 30, + grossSavingGBP: 0, + extraDrivingCostGBP: 0, + extraDistanceMiles: 0, + netSavingGBP: 0, + status: .baseline + ) + XCTAssertEqual(baseline.capsuleHeadline(), "Closest station · your baseline") + + let saving = FuelSavingsResult( + fillLitres: 30, + grossSavingGBP: 2.40, + extraDrivingCostGBP: 1.40, + extraDistanceMiles: 5.2, + netSavingGBP: 1.00, + status: .worthTheDrive, + distanceBasis: .straightLine + ) + XCTAssertEqual(saving.capsuleHeadline(), "Save £1.00 after travel (est.)") + + let worse = FuelSavingsResult( + fillLitres: 30, + grossSavingGBP: 1.40, + extraDrivingCostGBP: 1.70, + extraDistanceMiles: 6.7, + netSavingGBP: -0.31, + status: .notWorthTheDrive + ) + XCTAssertEqual(worse.capsuleHeadline(), "£0.31 worse off after travel") + } } diff --git a/Shared/FuelSavings.swift b/Shared/FuelSavings.swift index 3f54fd0..63bff78 100644 --- a/Shared/FuelSavings.swift +++ b/Shared/FuelSavings.swift @@ -100,6 +100,25 @@ enum FuelSavingsFormat { static func money(_ value: Double) -> String { String(format: "£%.2f", value) } + + /// A distance stored in MILES, formatted in the display unit. The figures + /// behind an estimate are stored in miles whatever the user's unit, so this + /// is the one place that conversion lives. + static func distance(_ miles: Double, unit: DistanceUnit) -> String { + // Convert miles→km FIRST, then let the unit format it. `unit.toKM` would + // convert by the CURRENT unit instead, leaving a km user's figure + // unlabelled and 1.6× too large. + unit.format(miles * FuelSavingsCalculator.kilometresPerMile) + } + + /// Row label for the extra driving a saving has to beat. The stored figure is + /// always MILES while the row's distance line is one-way, so the wording says + /// "more driving": the extra is an out-and-back figure and can legitimately + /// exceed the one-way distance, which read as a contradiction before. + static func extraTravelLabel(extraDistanceMiles: Double, unit: DistanceUnit) -> String? { + guard extraDistanceMiles > 0.05 else { return nil } + return "\(distance(extraDistanceMiles, unit: unit)) more driving than closest" + } } extension FuelSavingsResult { @@ -140,6 +159,15 @@ extension FuelSavingsResult { return "\(FuelSavingsFormat.money(abs(netSavingGBP))) worse off after travel\(estimate)" } } + + /// Row capsule copy: the headline, plus what the baseline row IS. Without the + /// suffix the baseline capsule just repeated the status word above it + /// ("Closest station" twice) and said nothing about the saving being measured + /// against it. + func capsuleHeadline() -> String { + let headline = headline(compact: true) + return status == .baseline ? "\(headline) · your baseline" : headline + } } /// One station's journey distance, plus how it was obtained. @@ -173,7 +201,18 @@ enum FuelSavingsError: Error, Equatable { } enum FuelSavingsCalculator { + /// 1 UK (imperial) gallon. Must never be the US gallon (3.785411784 L): + /// UK pumps are priced per litre and drivers quote UK MPG, so a US gallon + /// would understate every travel cost by 17%. static let imperialGallonLitres = 4.54609 + /// Exact mile↔kilometre factors (1 mile = 1.609344 km). Every figure the + /// savings maths keeps is in MILES, and every displayed or fallback + /// distance converts through these, so one quantity is never converted with + /// two different constants. The audit found 1.60934, 1609.34 and 0.621371 + /// mixed in the same path — a ~2 parts-per-million difference, immaterial to + /// the money, but three constants where there should be one. + static let kilometresPerMile = 1.609344 + static let metresPerMile = 1609.344 static let breakEvenToleranceGBP = 0.05 static let minimumMPG: Double = 5 static let maximumMPG: Double = 200