From c083ac6fdafc3d1c420cedf746125490f66901ae Mon Sep 17 00:00:00 2001 From: Sujoy Das Date: Sat, 5 Sep 2026 21:29:28 +0300 Subject: [PATCH 1/8] refactor: move the review form's commit path onto WineForm so tests can reach it --- Vinnota/Model/AppState.swift | 49 ++++++++++++++++++++++++++++++++++ Vinnota/Views/ReviewView.swift | 34 +++-------------------- 2 files changed, 52 insertions(+), 31 deletions(-) diff --git a/Vinnota/Model/AppState.swift b/Vinnota/Model/AppState.swift index beae545..9654167 100644 --- a/Vinnota/Model/AppState.swift +++ b/Vinnota/Model/AppState.swift @@ -119,6 +119,55 @@ struct WineForm { var isComplete: Bool { missingRequired.isEmpty } + /// Builds the bottle this form describes. Lives on the form rather than + /// inside the view's `save()` so the commit path — trimming, the blank-price + /// sentinel, the hand-entry flag — is reachable from tests. It was not, and + /// a mutation that dropped `.trimmed` went undetected. + func makeWine() -> Wine { + let wine = Wine( + producer: producer.trimmed, + name: name.trimmed, + vintage: vintage.trimmed, + region: region.trimmed, + grape: grape.trimmed, + shop: shop.trimmed, + price: price.isBlank ? nil : price.trimmed, + currency: currency, + labelPhoto: labelPhoto + ) + // Neither read off a label nor carrying one: the user typed it in. + wine.addedByHand = !recognized && labelPhoto == nil + return wine + } + + /// Writes this form over a bottle already in the book. Mirrors `makeWine` + /// field for field; the two must not drift. + func apply(to wine: Wine) { + wine.producer = producer.trimmed + wine.name = name.trimmed + wine.vintage = vintage.trimmed + wine.region = region.trimmed + wine.grape = grape.trimmed + wine.shop = shop.trimmed + wine.price = price.isBlank ? nil : price.trimmed + wine.currency = currency + wine.labelPhoto = labelPhoto + } + + /// The notes this form has collected, ready to attach to a saved bottle. + /// The typed note lands first, then anything dictated. + func makeNotes() -> [TastingNote] { + var out: [TastingNote] = [] + if !text.isBlank { + out.append(TastingNote(kind: .text, phase: .pre, text: text.trimmed)) + } + for dictated in notes { + out.append(TastingNote(kind: dictated.typed ? .text : .voice, phase: .pre, + text: dictated.text, when: dictated.when)) + } + return out + } + /// Loads an existing bottle back into the form for editing. Notes are left /// alone — they are their own records with their own affordances. init(editing wine: Wine) { diff --git a/Vinnota/Views/ReviewView.swift b/Vinnota/Views/ReviewView.swift index 31db9d5..accaf30 100644 --- a/Vinnota/Views/ReviewView.swift +++ b/Vinnota/Views/ReviewView.swift @@ -255,15 +255,7 @@ struct ReviewView: View { let f = app.form if let existing = app.editing { - existing.producer = f.producer.trimmed - existing.name = f.name.trimmed - existing.vintage = f.vintage.trimmed - existing.region = f.region.trimmed - existing.grape = f.grape.trimmed - existing.shop = f.shop.trimmed - existing.price = f.price.isBlank ? nil : f.price.trimmed - existing.currency = f.currency - existing.labelPhoto = f.labelPhoto + f.apply(to: existing) try? context.save() app.form = WineForm() @@ -274,29 +266,9 @@ struct ReviewView: View { return } - let wine = Wine( - producer: f.producer.trimmed, - name: f.name.trimmed, - vintage: f.vintage.trimmed, - region: f.region.trimmed, - grape: f.grape.trimmed, - shop: f.shop.trimmed, - price: f.price.isBlank ? nil : f.price.trimmed, - currency: f.currency, - labelPhoto: f.labelPhoto - ) - wine.addedByHand = !f.recognized && f.labelPhoto == nil + let wine = f.makeWine() context.insert(wine) - - // The typed note lands first, then anything dictated. - if !f.text.isEmpty { - let note = TastingNote(kind: .text, phase: .pre, text: f.text) - note.wine = wine - context.insert(note) - } - for dictated in f.notes { - let note = TastingNote(kind: dictated.typed ? .text : .voice, phase: .pre, - text: dictated.text, when: dictated.when) + for note in f.makeNotes() { note.wine = wine context.insert(note) } From ba05511d66c83ceaeaa42b6ef828dc20b2ded1df Mon Sep 17 00:00:00 2001 From: Sujoy Das Date: Sat, 5 Sep 2026 21:29:28 +0300 Subject: [PATCH 2/8] test: add unit test target covering validation, label parsing and the wine model --- Vinnota.xcodeproj/project.pbxproj | 106 ++ .../xcshareddata/xcschemes/Vinnota.xcscheme | 13 +- VinnotaTests/FormValidationTests.swift | 611 +++++++++ VinnotaTests/LabelScannerTests.swift | 663 ++++++++++ VinnotaTests/WineModelTests.swift | 1090 +++++++++++++++++ 5 files changed, 2482 insertions(+), 1 deletion(-) create mode 100644 VinnotaTests/FormValidationTests.swift create mode 100644 VinnotaTests/LabelScannerTests.swift create mode 100644 VinnotaTests/WineModelTests.swift diff --git a/Vinnota.xcodeproj/project.pbxproj b/Vinnota.xcodeproj/project.pbxproj index 89e5407..c2f88ec 100644 --- a/Vinnota.xcodeproj/project.pbxproj +++ b/Vinnota.xcodeproj/project.pbxproj @@ -8,6 +8,7 @@ /* Begin PBXFileReference section */ A100000000000000000001 /* Vinnota.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = Vinnota.app; sourceTree = BUILT_PRODUCTS_DIR; }; + A100000000000000000020 /* VinnotaTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = VinnotaTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; /* End PBXFileReference section */ /* Begin PBXFileSystemSynchronizedBuildFileExceptionSet section */ @@ -30,9 +31,21 @@ path = Vinnota; sourceTree = ""; }; + A100000000000000000021 /* VinnotaTests */ = { + isa = PBXFileSystemSynchronizedRootGroup; + path = VinnotaTests; + sourceTree = ""; + }; /* End PBXFileSystemSynchronizedRootGroup section */ /* Begin PBXFrameworksBuildPhase section */ + A100000000000000000024 /* Frameworks */ = { + isa = PBXFrameworksBuildPhase; + buildActionMask = 2147483647; + files = ( + ); + runOnlyForDeploymentPostprocessing = 0; + }; A100000000000000000003 /* Frameworks */ = { isa = PBXFrameworksBuildPhase; buildActionMask = 2147483647; @@ -47,6 +60,7 @@ isa = PBXGroup; children = ( A100000000000000000002 /* Vinnota */, + A100000000000000000021 /* VinnotaTests */, A100000000000000000004 /* Products */, ); sourceTree = ""; @@ -55,6 +69,7 @@ isa = PBXGroup; children = ( A100000000000000000001 /* Vinnota.app */, + A100000000000000000020 /* VinnotaTests.xctest */, ); name = Products; sourceTree = ""; @@ -82,8 +97,46 @@ productReference = A100000000000000000001 /* Vinnota.app */; productType = "com.apple.product-type.application"; }; + A100000000000000000022 /* VinnotaTests */ = { + isa = PBXNativeTarget; + buildConfigurationList = A100000000000000000025 /* Build configuration list for PBXNativeTarget "VinnotaTests" */; + buildPhases = ( + A100000000000000000023 /* Sources */, + A100000000000000000024 /* Frameworks */, + ); + buildRules = ( + ); + dependencies = ( + A100000000000000000028 /* PBXTargetDependency */, + ); + fileSystemSynchronizedGroups = ( + A100000000000000000021 /* VinnotaTests */, + ); + name = VinnotaTests; + productName = VinnotaTests; + productReference = A100000000000000000020 /* VinnotaTests.xctest */; + productType = "com.apple.product-type.bundle.unit-test"; + }; /* End PBXNativeTarget section */ +/* Begin PBXTargetDependency section */ + A100000000000000000028 /* PBXTargetDependency */ = { + isa = PBXTargetDependency; + target = A100000000000000000005 /* Vinnota */; + targetProxy = A100000000000000000029 /* PBXContainerItemProxy */; + }; +/* End PBXTargetDependency section */ + +/* Begin PBXContainerItemProxy section */ + A100000000000000000029 /* PBXContainerItemProxy */ = { + isa = PBXContainerItemProxy; + containerPortal = A10000000000000000000A /* Project object */; + proxyType = 1; + remoteGlobalIDString = A100000000000000000005; + remoteInfo = Vinnota; + }; +/* End PBXContainerItemProxy section */ + /* Begin PBXProject section */ A10000000000000000000A /* Project object */ = { isa = PBXProject; @@ -112,6 +165,7 @@ projectRoot = ""; targets = ( A100000000000000000005 /* Vinnota */, + A100000000000000000022 /* VinnotaTests */, ); }; /* End PBXProject section */ @@ -127,6 +181,13 @@ /* End PBXResourcesBuildPhase section */ /* Begin PBXSourcesBuildPhase section */ + A100000000000000000023 /* Sources */ = { + isa = PBXSourcesBuildPhase; + buildActionMask = 2147483647; + files = ( + ); + runOnlyForDeploymentPostprocessing = 0; + }; A100000000000000000006 /* Sources */ = { isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; @@ -238,6 +299,42 @@ }; name = Release; }; + A100000000000000000026 /* Debug */ = { + isa = XCBuildConfiguration; + buildSettings = { + BUNDLE_LOADER = "$(TEST_HOST)"; + CODE_SIGN_STYLE = Automatic; + CURRENT_PROJECT_VERSION = 1; + GENERATE_INFOPLIST_FILE = YES; + IPHONEOS_DEPLOYMENT_TARGET = 17.0; + MARKETING_VERSION = 1.0; + PRODUCT_BUNDLE_IDENTIFIER = com.vinnota.cellarbook.tests; + PRODUCT_NAME = "$(TARGET_NAME)"; + SWIFT_EMIT_LOC_STRINGS = NO; + SWIFT_VERSION = 5.0; + TARGETED_DEVICE_FAMILY = "1,2"; + TEST_HOST = "$(BUILT_PRODUCTS_DIR)/Vinnota.app/$(BUNDLE_EXECUTABLE_FOLDER_PATH)/Vinnota"; + }; + name = Debug; + }; + A100000000000000000027 /* Release */ = { + isa = XCBuildConfiguration; + buildSettings = { + BUNDLE_LOADER = "$(TEST_HOST)"; + CODE_SIGN_STYLE = Automatic; + CURRENT_PROJECT_VERSION = 1; + GENERATE_INFOPLIST_FILE = YES; + IPHONEOS_DEPLOYMENT_TARGET = 17.0; + MARKETING_VERSION = 1.0; + PRODUCT_BUNDLE_IDENTIFIER = com.vinnota.cellarbook.tests; + PRODUCT_NAME = "$(TARGET_NAME)"; + SWIFT_EMIT_LOC_STRINGS = NO; + SWIFT_VERSION = 5.0; + TARGETED_DEVICE_FAMILY = "1,2"; + TEST_HOST = "$(BUILT_PRODUCTS_DIR)/Vinnota.app/$(BUNDLE_EXECUTABLE_FOLDER_PATH)/Vinnota"; + }; + name = Release; + }; /* End XCBuildConfiguration section */ /* Begin XCConfigurationList section */ @@ -259,6 +356,15 @@ defaultConfigurationIsVisible = 0; defaultConfigurationName = Release; }; + A100000000000000000025 /* Build configuration list for PBXNativeTarget "VinnotaTests" */ = { + isa = XCConfigurationList; + buildConfigurations = ( + A100000000000000000026 /* Debug */, + A100000000000000000027 /* Release */, + ); + defaultConfigurationIsVisible = 0; + defaultConfigurationName = Release; + }; /* End XCConfigurationList section */ }; rootObject = A10000000000000000000A /* Project object */; diff --git a/Vinnota.xcodeproj/xcshareddata/xcschemes/Vinnota.xcscheme b/Vinnota.xcodeproj/xcshareddata/xcschemes/Vinnota.xcscheme index b42fe3b..893b1a7 100644 --- a/Vinnota.xcodeproj/xcshareddata/xcschemes/Vinnota.xcscheme +++ b/Vinnota.xcodeproj/xcshareddata/xcschemes/Vinnota.xcscheme @@ -14,7 +14,18 @@ - + + + + + + diff --git a/VinnotaTests/FormValidationTests.swift b/VinnotaTests/FormValidationTests.swift new file mode 100644 index 0000000..170e31f --- /dev/null +++ b/VinnotaTests/FormValidationTests.swift @@ -0,0 +1,611 @@ +import Foundation +import SwiftData +import Testing + +@testable import Vinnota + +// MARK: - Fixtures + +/// Strings that are dangerous in some *other* system (SQL, a filesystem path, +/// a `printf`-style formatter, an HTML renderer). Vinnota stores them in +/// SwiftData and renders them through SwiftUI `Text`, both of which are +/// value-safe, so the property worth locking down is that they are treated as +/// ordinary text: they pass validation, survive a store round-trip byte for +/// byte, and never get interpreted. +private let hostileButOrdinaryText: [String] = [ + "'; DROP TABLE ZWINE; --", + "Robert'); DROP TABLE Students;--", + "\" OR 1=1 --", + "../../etc/passwd", + "..\\..\\Windows\\System32", + "/dev/null", + "%@ %n %s %d %1$@", + "100%% sure", + "", + "", + "{{7*7}}", + "${jndi:ldap://evil/x}", + "Château\u{0301} Margaux \u{1F377}", // combining mark + emoji + "\u{202E}gnitset", // right-to-left override +] + +/// A blank producer keeps `Add to the book` disabled. These are the inputs a +/// user can actually produce with a keyboard or a paste that *look* filled. +private let blankLookingProducers: [String] = [ + " ", + " ", + "\t", + "\n", + "\r\n", + "\t \n \r ", + "\u{00A0}", // NO-BREAK SPACE — what a paste from the web gives you + "\u{00A0}\u{00A0} \t", + "\u{200A}", // HAIR SPACE + "\u{2028}", // LINE SEPARATOR + "\u{2029}", // PARAGRAPH SEPARATOR + "\u{3000}", // IDEOGRAPHIC SPACE + "\u{000B}", // VERTICAL TAB + "\u{000C}", // FORM FEED + "\u{0085}", // NEXT LINE + "\u{200B}", // ZERO WIDTH SPACE — see the finding below +] + +private func form(producer: String) -> WineForm { + var f = WineForm() + f.producer = producer + return f +} + +// MARK: - Required fields + +@Suite("WineForm required fields") +struct RequiredFieldTests { + + /// An untouched form is missing exactly one thing, and the copy is the copy + /// the save bar prints. If someone renames the phrase, the hint that reads + /// "Add the producer first." silently changes with it. + @Test func emptyFormIsMissingOnlyTheProducer() { + let f = WineForm() + #expect(f.missingRequired == ["the producer"]) + #expect(f.isComplete == false) + } + + @Test func producerAloneCompletesTheForm() { + let f = form(producer: "Giuseppe Rinaldi") + #expect(f.missingRequired.isEmpty) + #expect(f.isComplete) + } + + /// Year, region and shop are deliberately optional — the form is filled + /// standing in a shop aisle. Re-requiring any of them must fail here. + @Test func yearRegionAndShopAreOptional() { + var f = form(producer: "Giuseppe Rinaldi") + f.vintage = "" + f.region = "" + f.shop = "" + f.grape = "" + f.name = "" + f.price = "" + f.text = "" + #expect(f.isComplete, "only the producer may gate a save") + #expect(f.missingRequired.isEmpty) + } + + /// The mirror image: filling an optional field must not rescue a form whose + /// producer is empty. Catches a regression that swaps which field is checked. + @Test(arguments: [ + \WineForm.vintage, \WineForm.region, \WineForm.shop, + \WineForm.grape, \WineForm.name, \WineForm.price, \WineForm.text, + ]) + func optionalFieldAloneDoesNotCompleteTheForm(field: WritableKeyPath) { + var f = WineForm() + f[keyPath: field] = "something" + #expect(f.missingRequired == ["the producer"]) + #expect(f.isComplete == false) + } + + /// The real bug class: " " looks filled in the field but names nothing. + @Test(arguments: blankLookingProducers) + func whitespaceOnlyProducerDoesNotSatisfyValidation(producer: String) { + let f = form(producer: producer) + #expect(f.producer.isEmpty == false, "precondition: the field is not literally empty") + #expect(f.missingRequired == ["the producer"]) + #expect(f.isComplete == false) + } + + /// Which invisible characters `.whitespacesAndNewlines` actually catches, + /// asserted rather than assumed: + /// + /// CAUGHT (treated as blank): U+0009 tab, U+000A/U+000D newline, + /// U+000B vertical tab, U+000C form feed, U+0085 next line, every Unicode + /// `Zs` space (U+0020, U+00A0 no-break, U+2000–U+200A, U+202F, U+205F, + /// U+3000), U+2028/U+2029 separators, and — surprisingly — U+200B ZERO + /// WIDTH SPACE, which Foundation includes in `.whitespaces` even though its + /// Unicode category is `Cf`, not `Zs`. + /// + /// NOT CAUGHT (passes validation): see the finding below. + @Test(arguments: [ + "\u{0000}", // NUL + "\u{0007}", // BEL + "\u{001B}", // ESC + "\u{FEFF}", // ZERO WIDTH NO-BREAK SPACE / BOM + "\u{180E}", // MONGOLIAN VOWEL SEPARATOR + "\u{2060}", // WORD JOINER + ]) + func invisibleCharactersThatSlipPastValidation(producer: String) { + let f = form(producer: producer) + // FINDING: `String.isBlank` only trims `.whitespacesAndNewlines`, so + // control characters (U+0000, U+0007, U+001B) and the zero-width + // formatting characters outside Foundation's whitespace set (U+FEFF, + // U+180E, U+2060) count as a filled producer. A bottle can be saved + // with a producer that renders as nothing at all: the card, the detail + // header and the search index all show an empty name. Reachable by + // pasting from a spreadsheet or a BOM-prefixed CSV cell. + // Asserting the real behaviour, not the desired one — a fix would be to + // trim `.controlCharacters` and `.init(charactersIn: "\u{FEFF}\u{2060}")` + // as well, in Vinnota/Model/AppState.swift. + #expect(f.isComplete, "documented current behaviour, not the desired one") + #expect(f.missingRequired.isEmpty) + #expect(f.producer.trimmed.isEmpty == false) + } + + /// A producer with real content is not damaged by the padding around it. + @Test func paddingAroundRealContentIsIgnoredByValidationAndStrippedByTrimming() { + let f = form(producer: " \n\t Domaine Leflaive \u{00A0} ") + #expect(f.isComplete) + #expect(f.producer.trimmed == "Domaine Leflaive") + } + + /// Interior whitespace is content, not padding — `trimmed` must not collapse it. + @Test func interiorWhitespaceIsPreserved() { + #expect(" Ch. Le Puy Barthélemy ".trimmed == "Ch. Le Puy Barthélemy") + #expect("A\nB".trimmed == "A\nB") + } +} + +// MARK: - Loading a saved bottle back into the form + +/// `WineForm(editing:)` is the other half of the review-form round trip and is +/// real, directly reachable app code (Vinnota/Model/AppState.swift). Tapping +/// "Edit the bottle" runs it, and whatever it produces is what `save()` writes +/// back — so a field it drops is a field the user silently loses on their next +/// save. +@MainActor +@Suite("WineForm(editing:)") +struct EditRoundTripTests { + + private func saved() -> Wine { + let wine = Wine(producer: "Giuseppe Rinaldi", name: "Brunate", vintage: "2019", + region: "Piemonte, IT", grape: "Nebbiolo", shop: "Vino e Sapori", + price: "42.00", currency: .SEK) + return wine + } + + /// Every field the edit screen exposes must survive load → save unchanged. + /// Catches a regression that drops or crosses a field in the initialiser. + @Test func everyEditableFieldIsCarriedBackIntoTheForm() { + let wine = saved() + let f = WineForm(editing: wine) + + #expect(f.producer == "Giuseppe Rinaldi") + #expect(f.name == "Brunate") + #expect(f.vintage == "2019") + #expect(f.region == "Piemonte, IT") + #expect(f.grape == "Nebbiolo") + #expect(f.shop == "Vino e Sapori") + #expect(f.price == "42.00") + #expect(f.currency == .SEK, "the bottle's currency, not the .EUR default") + #expect(f.isComplete, "a saved bottle always has a producer") + } + + /// A bottle loaded for editing must not be re-labelled as hand-entered or + /// as an OCR reading; `recognized` gates the "Read off the label" banner. + @Test func editingDoesNotClaimTheFormWasScanned() { + #expect(WineForm(editing: saved()).recognized == false) + } + + /// The em dash is the legacy "no cuvée" marker. It must come back as an + /// empty field, so the user sees the "Cuvée or grape" placeholder rather + /// than a literal dash they have to delete by hand. + @Test func legacyEmDashNameLoadsAsAnEmptyCuveeField() { + let wine = Wine(producer: "Rinaldi", name: "—", vintage: "", region: "", + grape: "", shop: "") + #expect(WineForm(editing: wine).name == "") + } + + /// Only the exact em-dash sentinel is stripped — a real cuvée that merely + /// contains a dash is content and must survive. + @Test(arguments: ["—", "-", "– ", "Clos du —", "——", "Côte-Rôtie"]) + func onlyTheBareEmDashIsTreatedAsNoCuvee(name: String) { + let wine = Wine(producer: "Rinaldi", name: name, vintage: "", region: "", + grape: "", shop: "") + let expected = name == "—" ? "" : name + #expect(WineForm(editing: wine).name == expected) + } + + /// `Wine.price` is optional, `WineForm.price` is not. A priceless bottle + /// must load as "" — the field renders `price` directly, so a stray + /// "nil" or a crash would both surface here. + @Test func aBottleWithNoPriceLoadsAsAnEmptyPriceField() { + let wine = Wine(producer: "Rinaldi", name: "", vintage: "", region: "", + grape: "", shop: "", price: nil) + let f = WineForm(editing: wine) + #expect(f.price == "") + #expect(f.price.isBlank, "so the commit stores nil again, not \"\"") + } + + /// Load → commit → load must be a fixed point: a second trip through the + /// edit screen may not mutate anything. This is the property that a + /// half-applied trim or a dropped field actually breaks. + @Test func loadingAnEditedFormAgainChangesNothing() { + let wine = saved() + let first = WineForm(editing: wine) + + // Apply the form back, as save()'s `existing.… = ….trimmed` block does. + wine.producer = first.producer.trimmed + wine.name = first.name.trimmed + wine.vintage = first.vintage.trimmed + wine.region = first.region.trimmed + wine.grape = first.grape.trimmed + wine.shop = first.shop.trimmed + wine.price = first.price.isBlank ? nil : first.price.trimmed + wine.currency = first.currency + + let second = WineForm(editing: wine) + #expect(second.producer == first.producer) + #expect(second.name == first.name) + #expect(second.vintage == first.vintage) + #expect(second.region == first.region) + #expect(second.grape == first.grape) + #expect(second.shop == first.shop) + #expect(second.price == first.price) + #expect(second.currency == first.currency) + } + + /// A saved bottle whose producer is whitespace (possible only via a write + /// path that skipped validation) must re-block the save, not sail through. + @Test func aBlankProducerOnAStoredBottleStillBlocksTheEditSave() { + let wine = Wine(producer: " ", name: "", vintage: "", region: "", + grape: "", shop: "") + let f = WineForm(editing: wine) + #expect(f.isComplete == false) + #expect(f.missingRequired == ["the producer"]) + } +} + +// MARK: - The missing-field sentence + +@Suite("String.list grammar") +struct MissingFieldProseTests { + + @Test func zeroItemsProduceNothing() { + #expect(String.list([]) == "") + } + + @Test func oneItemIsItself() { + #expect(String.list(["the producer"]) == "the producer") + } + + @Test func twoItemsAreJoinedByAnd() { + #expect(String.list(["the producer", "the year"]) == "the producer and the year") + } + + @Test func threeItemsUseCommasAndAFinalAnd() { + #expect(String.list(["the producer", "the year", "the region"]) + == "the producer, the year and the region") + } + + @Test func fourItemsKeepTheSameShape() { + #expect(String.list(["a", "b", "c", "d"]) == "a, b, c and d") + } + + /// The sentence the save bar and the toast both build. + @Test func theHintReadsAsASentenceForAnEmptyForm() { + let hint = "Add \(String.list(WineForm().missingRequired)) first." + #expect(hint == "Add the producer first.") + } + + /// Format specifiers inside a field name are interpolated, not formatted. + @Test func listDoesNotInterpretFormatSpecifiers() { + #expect(String.list(["%@", "%n"]) == "%@ and %n") + #expect("Add \(String.list(["%d %s"])) first." == "Add %d %s first.") + } +} + +// MARK: - What reaches the store + +/// Covers the real commit path. `ReviewView.save()` used to inline its field +/// mapping, so this suite could only mirror it — and a mutation that dropped +/// `.trimmed` from the real `save()` left the suite green. The mapping now +/// lives on `WineForm` (`makeWine`, `apply(to:)`, `makeNotes`) and `save()` +/// delegates to it, so these tests exercise the code that actually runs. +@MainActor +@Suite("Commit rules (WineForm.makeWine / apply)") +struct SaveCommitTests { + + /// Delegates to the app's own commit path — deliberately a one-line + /// forwarder so these tests cannot drift from what `save()` does. + private func commit(_ form: WineForm, into context: ModelContext) -> Wine { + let wine = form.makeWine() + context.insert(wine) + for note in form.makeNotes() { + note.wine = wine + context.insert(note) + } + return wine + } + + private func newContext() throws -> ModelContext { + let container = try ModelContainer( + for: Schema([Wine.self, TastingNote.self]), + configurations: [ModelConfiguration(isStoredInMemoryOnly: true)] + ) + return ModelContext(container) + } + + + @Test func everyTextFieldIsTrimmedBeforeItIsStored() throws { + let context = try newContext() + var f = WineForm() + f.producer = " Giuseppe Rinaldi\n" + f.name = "\tBrunate " + f.vintage = " 2019 " + f.region = "\u{00A0}Piemonte, IT " + f.grape = " Nebbiolo\t" + f.shop = " Vino e Sapori " + f.price = " 42.00 " + + let wine = commit(f, into: context) + try context.save() + + let stored = try #require(try context.fetch(FetchDescriptor()).first) + #expect(stored.producer == "Giuseppe Rinaldi") + #expect(stored.name == "Brunate") + #expect(stored.vintage == "2019") + #expect(stored.region == "Piemonte, IT") + #expect(stored.grape == "Nebbiolo") + #expect(stored.shop == "Vino e Sapori") + #expect(stored.price == "42.00") + #expect(stored.persistentModelID == wine.persistentModelID) + + // Nothing round-tripped with padding still attached. + for value in [stored.producer, stored.name, stored.vintage, + stored.region, stored.grape, stored.shop, stored.price ?? ""] { + #expect(value == value.trimmed) + } + } + + /// A blank price becomes `nil`, not `""` — `hasPrice` tests `isEmpty == false`, + /// so a whitespace price would light up the price line with a bare currency symbol. + @Test(arguments: ["", " ", "\t\n", "\u{00A0}"]) + func blankPriceIsStoredAsNil(price: String) throws { + let context = try newContext() + var f = form(producer: "Rinaldi") + f.price = price + + let wine = commit(f, into: context) + try context.save() + + #expect(wine.price == nil) + #expect(wine.hasPrice == false) + } + + @Test func nonBlankPriceIsStoredTrimmed() throws { + let context = try newContext() + var f = form(producer: "Rinaldi") + f.price = " 18.50 " + + let wine = commit(f, into: context) + try context.save() + + #expect(wine.price == "18.50") + #expect(wine.hasPrice) + } + + /// Why the trimming in `save()` is load-bearing rather than cosmetic: + /// `subtitle` and `eyebrow` filter on `isEmpty`, not `isBlank`. + @Test func derivedDisplayStringsDependOnTheCommitHavingTrimmed() throws { + let context = try newContext() + var f = form(producer: "Rinaldi") + f.name = " Brunate " + f.region = " " // whitespace-only region, e.g. a stray space typed in + f.grape = " Nebbiolo " + + let wine = commit(f, into: context) + try context.save() + + #expect(wine.subtitle == "Brunate") + #expect(wine.eyebrow == "Nebbiolo") + + // FINDING (latent, currently masked by `save()`): `Wine.subtitle` and + // `Wine.eyebrow` in Vinnota/Model/Wine.swift drop empty components with + // `filter { !$0.isEmpty }`, which a whitespace-only string passes. Any + // future write path that skips `.trimmed` produces a dangling " · ". + let untrimmed = Wine(producer: "Rinaldi", name: "Brunate", vintage: "", + region: " ", grape: "Nebbiolo", shop: "") + #expect(untrimmed.subtitle == "Brunate · ") + #expect(untrimmed.eyebrow == "Nebbiolo · ") + } + + /// The em dash is the legacy "no cuvée" marker; a padded one must still + /// collapse to nothing rather than printing a stray dash. + @Test func paddedLegacyEmDashNameStillDisplaysAsNoCuvee() throws { + let context = try newContext() + var f = form(producer: "Rinaldi") + f.name = " — " + + let wine = commit(f, into: context) + try context.save() + + #expect(wine.name == "—") + #expect(wine.displayName == "") + #expect(wine.subtitle == "") + } +} + +// MARK: - Hostile-looking input is ordinary text + +@MainActor +@Suite("Injection-shaped input") +struct HostileInputTests { + + /// Delegates to the app's own commit path — deliberately a one-line + /// forwarder so these tests cannot drift from what `save()` does. + private func commit(_ form: WineForm, into context: ModelContext) -> Wine { + let wine = form.makeWine() + context.insert(wine) + for note in form.makeNotes() { + note.wine = wine + context.insert(note) + } + return wine + } + + private func newContext() throws -> ModelContext { + let container = try ModelContainer( + for: Schema([Wine.self, TastingNote.self]), + configurations: [ModelConfiguration(isStoredInMemoryOnly: true)] + ) + return ModelContext(container) + } + + /// A producer that looks like SQL, a path or a format string is a legal + /// producer: validation must accept it and trimming must not alter it. + @Test(arguments: hostileButOrdinaryText) + func hostileLookingTextIsValidAndUnaltered(value: String) { + var f = form(producer: value) + f.name = value + f.region = value + f.shop = value + #expect(f.isComplete) + #expect(f.missingRequired.isEmpty) + #expect(f.producer.trimmed == value, "no sanitising, no escaping — stored verbatim") + #expect(f.producer.isBlank == false) + } + + /// Round-trips through SwiftData byte for byte, in every text field. + @Test(arguments: hostileButOrdinaryText) + func hostileLookingTextRoundTripsThroughTheStore(value: String) throws { + let context = try newContext() + let wine = Wine(producer: value.trimmed, name: value.trimmed, vintage: value.trimmed, + region: value.trimmed, grape: value.trimmed, shop: value.trimmed, + price: value.trimmed) + context.insert(wine) + let note = TastingNote(kind: .text, phase: .pre, text: value) + note.wine = wine + context.insert(note) + try context.save() + + let stored = try #require(try context.fetch(FetchDescriptor()).first) + #expect(stored.producer == value) + #expect(stored.name == value) + #expect(stored.region == value) + #expect(stored.shop == value) + #expect(stored.price == value) + #expect(Array(stored.producer.unicodeScalars) == Array(value.unicodeScalars)) + #expect(stored.notes.count == 1) + #expect(stored.notes.first?.text == value) + } + + /// The one that would matter if SwiftData concatenated instead of binding: + /// a `DROP TABLE` producer must come back as one row, with the neighbours + /// still there. + @Test func sqlShapedProducerIsBoundAsAValueNotExecuted() throws { + let context = try newContext() + let payload = "'; DROP TABLE ZWINE; --" + for name in ["Rinaldi", payload, "Vajra"] { + context.insert(Wine(producer: name, name: "", vintage: "", region: "", + grape: "", shop: "")) + } + try context.save() + + let hits = try context.fetch( + FetchDescriptor(predicate: #Predicate { $0.producer == payload }) + ) + #expect(hits.count == 1) + #expect(hits.first?.producer == payload) + #expect(try context.fetch(FetchDescriptor()).count == 3, + "the other rows — and the table — survive") + } + + /// Search takes the same text and must find it, not choke on it. + @Test(arguments: hostileButOrdinaryText) + func searchMatchesHostileLookingTextLiterally(value: String) { + let wine = Wine(producer: value, name: "", vintage: "", region: "", + grape: "", shop: "") + #expect(wine.matches(query: value)) + #expect(wine.matches(query: "")) + #expect(wine.matches(query: " ")) + #expect(wine.matches(query: "definitely-not-in-there") == false) + } + + /// `Wine.matches` trims `.whitespaces` only, while every other blank check + /// in the app uses `.whitespacesAndNewlines`. So a space-only query is + /// "empty" (show everything) but a newline-only query is a literal search + /// for "\n" and hides the entire cellar. + @Test func aNewlineOnlyQueryIsNotTreatedAsAnEmptyQuery() { + let wine = Wine(producer: "Rinaldi", name: "", vintage: "", region: "", + grape: "", shop: "") + #expect(wine.matches(query: " \t "), "spaces and tabs mean no filter") + + // FINDING: asserting the real behaviour, not the desired one. In + // Vinnota/Model/Wine.swift `matches` uses `.whitespaces`; switching it + // to `.whitespacesAndNewlines` (or to `query.isBlank`) would make these + // agree with `isBlank` and with the space-only case above. + #expect(wine.matches(query: "\n") == false, "documented current behaviour") + #expect(wine.matches(query: "\r\n") == false, "documented current behaviour") + } +} + +// MARK: - Size + +@MainActor +@Suite("Oversized input") +struct LongInputTests { + + @Test func tenThousandCharacterProducerValidatesWithoutCrashing() { + let long = String(repeating: "Château ", count: 1_250) // 10_000 characters + #expect(long.count == 10_000) + + let f = form(producer: long) + #expect(f.isComplete) + #expect(f.missingRequired.isEmpty) + #expect(f.producer.trimmed.count == 9_999, "one trailing space is stripped") + } + + @Test func tenThousandSpacesIsStillAnEmptyProducer() { + let f = form(producer: String(repeating: " ", count: 10_000)) + #expect(f.isComplete == false) + #expect(f.missingRequired == ["the producer"]) + #expect(f.producer.trimmed.isEmpty) + } + + @Test func longInputBuriedInPaddingIsTrimmedCorrectly() { + let pad = String(repeating: " ", count: 5_000) + let body = String(repeating: "x", count: 10_000) + #expect((pad + body + pad).trimmed == body) + } + + @Test func longInputSurvivesTheStore() throws { + let container = try ModelContainer( + for: Schema([Wine.self, TastingNote.self]), + configurations: [ModelConfiguration(isStoredInMemoryOnly: true)] + ) + let context = ModelContext(container) + let long = String(repeating: "ü", count: 10_000) + context.insert(Wine(producer: long, name: "", vintage: "", region: "", + grape: "", shop: "")) + try context.save() + + let stored = try #require(try context.fetch(FetchDescriptor()).first) + #expect(stored.producer.count == 10_000) + #expect(stored.producer == long) + } + + @Test func longMissingFieldListDoesNotCrash() { + let many = (0..<1_000).map { "field \($0)" } + let sentence = String.list(many) + #expect(sentence.hasPrefix("field 0, field 1, ")) + #expect(sentence.hasSuffix(" and field 999")) + } +} diff --git a/VinnotaTests/LabelScannerTests.swift b/VinnotaTests/LabelScannerTests.swift new file mode 100644 index 0000000..0c400f4 --- /dev/null +++ b/VinnotaTests/LabelScannerTests.swift @@ -0,0 +1,663 @@ +import CoreGraphics +import Foundation +import Testing + +@testable import Vinnota + +// MARK: - Seam +// +// `LabelScanner.read(_:)` needs Vision and a camera frame, neither of which +// exists headlessly. `LabelScanner.parse(_:)` is already `internal static` and +// is the whole of the wine-specific logic: `read` does nothing but turn +// observations into `(text, height)` pairs and hand them over. So these tests +// drive `parse` directly through the seam that already exists — no access +// levels in the app were changed. + +/// A recognised line, as Vision would hand it over: the text plus the glyph +/// height that the ranking heuristic treats as prominence. +private func line(_ text: String, _ height: CGFloat) -> (text: String, height: CGFloat) { + (text: text, height: height) +} + +/// Parses a label given as tallest-to-shortest lines, which is how the +/// heuristic actually thinks about a bottle. +private func parse(_ pairs: [(String, CGFloat)]) -> LabelReading { + LabelScanner.parse(pairs.map { line($0.0, $0.1) }) +} + +/// Puts one line of interest next to an inert, taller producer line, so the +/// only thing that can move a field is the line under test. +private func parse(withNeutralProducer text: String) -> LabelReading { + parse([("Winery", 0.09), (text, 0.04)]) +} + +/// Seconds spent in `parse`, for the robustness budgets below. +private func elapsed(_ body: () -> Void) -> TimeInterval { + let start = Date() + body() + return Date().timeIntervalSince(start) +} + +/// Generous enough never to flake on a loaded CI machine, tight enough that a +/// quadratic blow-up or a runaway scan fails instead of wedging the suite. +private let budget: TimeInterval = 20 + +// MARK: - Vintage + +@Suite("LabelScanner · vintage extraction") +struct LabelScannerVintageTests { + + @Test("Plain four-digit vintages are read", arguments: [ + "2019", "1985", "2021", "1950", "2049", + ]) + func plainVintages(_ text: String) { + #expect(parse(withNeutralProducer: text).vintage == text) + } + + @Test("The plausible range is 1950...2049 inclusive", arguments: [ + ("1949", ""), ("1950", "1950"), ("2049", "2049"), ("2050", ""), + ("1899", ""), ("1888", ""), ("2100", ""), + ]) + func rangeBoundaries(_ text: String, _ expected: String) { + #expect(parse(withNeutralProducer: text).vintage == expected) + } + + /// The range is fixed, not relative to today, so a vintage in the near + /// future is accepted rather than rejected as impossible. That is the + /// right call for a wine app — futures and en-primeur labels are real — + /// and this pins it so it cannot drift silently. + @Test("A near-future year is accepted; an implausibly old one is not") + func futureAndAncient() { + #expect(parse(withNeutralProducer: "2049").vintage == "2049") + #expect(parse(withNeutralProducer: "1899").vintage == "") + } + + @Test("Alcohol percentages are not vintages", arguments: [ + "13.5%", "ALC. 13.5% BY VOL.", "ALC 14,5 % VOL", "12.5% ALC/VOL", "ALC 13% BY VOL", + ]) + func alcoholIsNotAVintage(_ text: String) { + #expect(parse(withNeutralProducer: text).vintage == "") + } + + @Test("Bottle volumes are not vintages", arguments: [ + "750 ML", "750ML", "e 750ml", "75 CL", "0.75 L", "1500 ML", "37.5 CL", + ]) + func volumeIsNotAVintage(_ text: String) { + #expect(parse(withNeutralProducer: text).vintage == "") + } + + @Test("Other three- and five-digit numbers are not vintages", arguments: [ + "750", "999", "12345", "1234567", "€ 24,95", "0000", + ]) + func otherNumbersAreNotVintages(_ text: String) { + #expect(parse(withNeutralProducer: text).vintage == "") + } + + /// FINDING (LabelScanner.swift:91): the year is matched anywhere in the + /// text, with nothing distinguishing a vintage from any other in-range + /// four-digit number on the label. A founding date — which appears on a + /// great many labels — is therefore read as the vintage. Not fixed here; + /// this asserts what the parser actually does today, and the Review screen + /// is where the user would have to catch it. + @Test("FINDING: a founding date is read as the vintage", arguments: [ + ("EST. 1978", "1978"), + ("ESTABLISHED 1978", "1978"), + ("FONDATA NEL 1961", "1961"), + ("LOT 2019", "2019"), + ]) + func foundingDateBecomesVintage(_ text: String, _ wrongVintage: String) { + #expect(parse(withNeutralProducer: text).vintage == wrongVintage) + } + + /// FINDING (LabelScanner.swift:91): a magnum-scale volume that happens to + /// land in the plausible range is read as a vintage, where 750 ML and + /// 1500 ML are correctly ignored. + @Test("FINDING: a volume inside the plausible range is read as a vintage") + func inRangeVolumeBecomesVintage() { + #expect(parse(withNeutralProducer: "1500 ML").vintage == "") + #expect(parse(withNeutralProducer: "2000 ML").vintage == "2000") + } + + @Test("A year embedded in a longer phrase is still extracted", arguments: [ + "Vintage 2019", "Anno 2019", "HARVESTED IN 2019 BY HAND", + ]) + func embeddedYear(_ text: String) { + #expect(parse(withNeutralProducer: text).vintage == "2019") + } + + @Test("Serif look-alike glyphs are corrected when two real digits remain", arguments: [ + ("2O19", "2019"), ("2OI9", "2019"), ("2Ol9", "2019"), + ("202I", "2021"), ("I999", "1999"), ("l985", "1985"), + ]) + func confusablesCorrected(_ text: String, _ expected: String) { + #expect(parse(withNeutralProducer: text).vintage == expected) + } + + /// The two-real-digit guard is what stops ordinary four-letter words from + /// being folded into years, and it is the only thing standing between the + /// confusable pass and nonsense vintages. + @Test("Four-letter words are not folded into years", arguments: [ + "BOOK", "LOOK", "POOL", "GOOD", "SOIL", "ZOI9", "OZIO", "GOLD", + ]) + func wordsAreNotYears(_ text: String) { + #expect(parse(withNeutralProducer: text).vintage == "") + } + + @Test("A label with no year at all leaves the vintage blank") + func noYearAnywhere() { + let r = parse([("VIETTI", 0.09), ("PERBACCO", 0.06), ("NEBBIOLO", 0.04)]) + #expect(r.vintage == "") + #expect(r.producer == "Vietti") + } + + /// The line is consumed only when the year is essentially all of it + /// (`count <= 6`), which decides whether it can go on to be picked as the + /// cuvée. + @Test("A bare year line is consumed; a wordy one stays in play as a cuvée") + func yearLineConsumption() { + let bare = parse([("Winery", 0.09), ("2019", 0.04), ("Cuvée Blanche", 0.06)]) + #expect(bare.vintage == "2019") + #expect(bare.name == "Cuvée Blanche") + + let wordy = parse([("Winery", 0.09), ("Vintage 2019", 0.04)]) + #expect(wordy.vintage == "2019") + #expect(wordy.name == "Vintage 2019") + } +} + +// MARK: - Field assignment + +@Suite("LabelScanner · producer, cuvée and region") +struct LabelScannerFieldTests { + + /// A Bordeaux label in the order it appears on the bottle: the château + /// name is the tallest text, the appellation repeats below it, and the + /// legal print is the smallest. + @Test("Bordeaux: tallest line is the producer, next is the cuvée") + func bordeaux() { + let r = parse([ + ("CHÂTEAU MARGAUX", 0.09), + ("MARGAUX", 0.06), + ("2015", 0.05), + ("GRAND VIN DE BORDEAUX", 0.03), + ("MIS EN BOUTEILLE AU CHÂTEAU", 0.02), + ("13.5% ALC/VOL", 0.015), + ("750 ML", 0.015), + ]) + #expect(r.producer == "Château Margaux") + #expect(r.name == "Margaux") + #expect(r.vintage == "2015") + #expect(r.region == "Bordeaux") + #expect(r.recognized) + } + + @Test("Barolo: the appellation goes to region, not to producer") + func barolo() { + let r = parse([ + ("BAROLO", 0.08), + ("Giacomo Conterno", 0.06), + ("Cascina Francia", 0.05), + ("2019", 0.04), + ("PIEMONTE, IT", 0.03), + ("DOCG", 0.03), + ("14% VOL", 0.01), + ]) + #expect(r.producer == "Giacomo Conterno") + #expect(r.name == "Cascina Francia") + #expect(r.region == "Barolo") + #expect(r.vintage == "2019") + } + + /// The appellation is consumed by the region rule even when it is the + /// tallest text on the label, so the producer falls through to the next + /// line down. This is the ranking assumption working as documented. + @Test("A region taller than the producer still lands in region") + func regionOutranksHeight() { + let r = parse([("BAROLO", 0.10), ("Vietti", 0.05), ("2019", 0.03)]) + #expect(r.region == "Barolo") + #expect(r.producer == "Vietti") + #expect(r.name == "") + } + + @Test("Burgundy: producer and cuvée survive a wall of legal boilerplate") + func burgundy() { + let r = parse([ + ("DOMAINE LEFLAIVE", 0.09), + ("PULIGNY-MONTRACHET", 0.06), + ("1ER CRU LES PUCELLES", 0.05), + ("2018", 0.04), + ("APPELLATION PULIGNY-MONTRACHET 1ER CRU CONTRÔLÉE", 0.02), + ("PRODUCT OF FRANCE", 0.02), + ("750ML", 0.015), + ("ALC 13% BY VOL", 0.015), + ]) + #expect(r.producer == "Domaine Leflaive") + #expect(r.name == "Puligny-Montrachet") + #expect(r.vintage == "2018") + // Puligny-Montrachet is not on the appellation list, so nothing on a + // white-Burgundy label matches and the region is simply left blank. + #expect(r.region == "") + } + + @Test("German label: region, grape, producer and cuvée all separate out") + func germanLabel() { + let r = parse([ + ("WEINGUT KELLER", 0.08), + ("ABTSERDE", 0.06), + ("RIESLING GG", 0.05), + ("2019", 0.04), + ("RHEINHESSEN", 0.03), + ]) + #expect(r.producer == "Weingut Keller") + #expect(r.name == "Abtserde") + #expect(r.grape == "Riesling") + #expect(r.region == "Rheinhessen") + #expect(r.vintage == "2019") + } + + @Test("Rioja: an all-caps label is title-cased for display") + func rioja() { + let r = parse([ + ("BODEGAS MUGA", 0.09), + ("RESERVA", 0.06), + ("RIOJA", 0.05), + ("2018", 0.04), + ("TEMPRANILLO", 0.03), + ]) + #expect(r.producer == "Bodegas Muga") + #expect(r.name == "Reserva") + #expect(r.region == "Rioja") + #expect(r.grape == "Tempranillo") + } + + @Test("Accented appellations match and keep their diacritics") + func accentedRegions() { + let rhone = parse([("E. GUIGAL", 0.09), ("CÔTES DU RHÔNE", 0.06), ("2020", 0.04)]) + #expect(rhone.region == "Côtes du Rhône") + #expect(rhone.producer == "E. Guigal") + + let priorat = parse([("MAS DOIX", 0.09), ("PRIORAT", 0.05), ("2017", 0.04)]) + #expect(priorat.region == "Priorat") + } + + /// OCR routinely drops accents; the region list is matched + /// diacritic-insensitively but reports the canonical spelling. + @Test("An unaccented misread still snaps to the accented appellation") + func diacriticInsensitiveRegion() { + #expect(parse([("Producteur", 0.09), ("COTES DU RHONE", 0.05)]).region == "Côtes du Rhône") + } + + @Test("A composed vs. decomposed accent parses identically") + func unicodeNormalisation() { + let composed = parse([("CHÂTEAU MARGAUX", 0.09), ("2015", 0.04)]) + let decomposed = parse([("CHA\u{0302}TEAU MARGAUX", 0.09), ("2015", 0.04)]) + #expect(composed.producer == decomposed.producer) + #expect(composed.vintage == decomposed.vintage) + } + + @Test("A misread place name snaps onto the known appellation") + func snapToKnownRegion() { + let r = parse([("Weingut Keller", 0.09), ("RIEINIESSEN, DE", 0.05), ("2019", 0.04)]) + #expect(r.region == "Rheinhessen, DE") + } + + @Test("A place-plus-country-code line keeps the code upper-cased") + func placeAndCountryCode() { + #expect(parse([("Opus One", 0.09), ("napa valley, ca", 0.05)]).region == "Napa Valley, CA") + #expect(parse([("Vietti", 0.09), ("PIEMONTE, IT", 0.05)]).region == "Piemonte, IT") + } + + /// FINDING (LabelScanner.swift:194): the "Place, XX" shape is matched + /// before the appellation list and consumes the line unconditionally, so a + /// producer written with a country suffix — a common way to print an + /// importer or estate line — is taken for a region and the producer is + /// left empty. + @Test("FINDING: a producer written 'Name, XX' is misread as a region") + func producerWithCountrySuffixBecomesRegion() { + let r = parse([("Weingut Keller, DE", 0.09), ("2019", 0.04)]) + #expect(r.region == "Weingut Keller, DE") + #expect(r.producer == "") + #expect(r.name == "") + } + + /// FINDING (LabelScanner.swift:210-221): boilerplate is matched as an + /// unanchored substring, and the list holds tokens as short as "cl", + /// "ml", "alc", "vol" and "doc". Those appear inside ordinary producer + /// names, so real estates are silently discarded from the producer + /// ranking. "Clos …" and "Vol…" are not edge cases — they are two of the + /// most common openings in French, Spanish and Italian wine. + @Test("FINDING: producer names containing a boilerplate token are discarded", arguments: [ + "CLOS MOGADOR", // "cl" + "Clos des Papes", // "cl" + "VOLNAY", // "vol" + "Castello di Volpaia", // "vol" + "Malcolm Wines", // "alc" + "Docteur Wines", // "doc" + ]) + func realProducersReadAsBoilerplate(_ producer: String) { + let r = parse([(producer, 0.09), ("2019", 0.04)]) + #expect(r.producer == "") + #expect(r.name == "") + } + + @Test("Producers without a boilerplate substring survive", arguments: [ + ("Domaine Leflaive", "Domaine Leflaive"), + ("Emilio Moro", "Emilio Moro"), + ("La Spinetta", "La Spinetta"), + ("Vietti", "Vietti"), + ]) + func ordinaryProducersSurvive(_ input: String, _ expected: String) { + #expect(parse([(input, 0.09), ("2019", 0.04)]).producer == expected) + } + + @Test("Genuine boilerplate never becomes the producer", arguments: [ + "PRODUCT OF FRANCE", "CONTAINS SULFITES", "MIS EN BOUTEILLE AU DOMAINE", + "ESTATE BOTTLED", "GOVERNMENT WARNING", "APPELLATION CONTRÔLÉE", + "IMBOTTIGLIATO ALL'ORIGINE", "750 ML", "ALC 13.5% BY VOL", + ]) + func boilerplateIsNeverTheProducer(_ text: String) { + #expect(parse([(text, 0.09)]).producer == "") + } + + @Test("A grape line is consumed when it is little more than the variety") + func grapeConsumption() { + let bare = parse([("Domaine X", 0.09), ("Pinot Noir", 0.05), ("2019", 0.04)]) + #expect(bare.grape == "Pinot Noir") + #expect(bare.name == "") + + // Longer than variety + 4, so it stays and can still be the cuvée. + let wordy = parse([("Domaine X", 0.09), ("Pinot Noir Reserve Bottling", 0.05), ("2019", 0.04)]) + #expect(wordy.grape == "Pinot Noir") + #expect(wordy.name == "Pinot Noir Reserve Bottling") + + // The cut is at variety + 4 characters. "Riesling" is 8, so 11 is + // consumed and 13 is not — a pair that straddles the threshold, which + // the two cases above (exactly equal, and far over) leave unpinned. + let justUnder = parse([("Weingut X", 0.09), ("Riesling GG", 0.05), ("2019", 0.04)]) + #expect(justUnder.grape == "Riesling") + #expect(justUnder.name == "") + + let justOver = parse([("Weingut X", 0.09), ("Riesling Sekt", 0.05), ("2019", 0.04)]) + #expect(justOver.grape == "Riesling") + #expect(justOver.name == "Riesling Sekt") + } +} + +// MARK: - The `recognized` flag + +@Suite("LabelScanner · the recognized flag") +struct LabelScannerRecognizedTests { + + /// `recognized` drives the Review screen's "Read off the label" + /// confirmation, so it must be false exactly when the parse produced + /// nothing worth confirming. + @Test("Empty input is not recognized") + func emptyInput() { + let r = parse([]) + #expect(!r.recognized) + #expect(r.producer.isEmpty) + #expect(r.vintage.isEmpty) + } + + @Test("A single legible line is recognized") + func singleLine() { + let r = parse([("CHÂTEAU MARGAUX", 0.09)]) + #expect(r.recognized) + #expect(r.producer == "Château Margaux") + } + + @Test("A year alone is enough to count as recognized") + func yearAlone() { + let r = parse([("2019", 0.05)]) + #expect(r.recognized) + #expect(r.vintage == "2019") + #expect(r.producer == "") + } + + @Test("Input that yields neither a producer nor a vintage is not recognized", arguments: [ + [("750", 0.05), ("13.5", 0.04), ("12345", 0.03)], + [("!!!!!!", 0.05), ("...---...", 0.04)], + [("PRODUCT OF FRANCE", 0.05), ("CONTAINS SULFITES", 0.04)], + [("", 0.05), ("", 0.04)], + [(" ", 0.05), ("\t\n", 0.04)], + ] as [[(String, CGFloat)]]) + func notRecognized(_ lines: [(String, CGFloat)]) { + #expect(!parse(lines).recognized) + } + + /// FINDING (LabelScanner.swift:86): `recognized` is derived from the + /// producer and the vintage only. A label where the OCR read an + /// appellation but no estate name and no year is reported as unrecognised + /// even though `region` is populated, so the Review screen drops its + /// "Read off the label" state while still showing a field that was in + /// fact read off the label. + @Test("FINDING: a region-only read is reported as unrecognized") + func regionOnlyIsNotRecognized() { + let r = parse([("BAROLO", 0.05)]) + #expect(r.region == "Barolo") + #expect(!r.recognized) + } + + /// The same holds for a grape-only read, and pinning the rule case by case + /// is what makes the two FINDINGs above legible. The expected flag is + /// written out rather than recomputed from the parser's own output — + /// re-deriving it from `producer`/`vintage` would just restate line 86 and + /// would hold however badly the fields themselves were filled in. + @Test("recognized is exactly 'a producer or a vintage was found'", arguments: [ + ([("CHÂTEAU MARGAUX", 0.09), ("2015", 0.04)], true), // both + ([("2019", 0.05)], true), // vintage only + ([("Vietti", 0.09)], true), // producer only + ([("BAROLO", 0.05)], false), // region only + ([("CHARDONNAY", 0.05)], false), // grape only + ([("750 ML", 0.05)], false), // boilerplate only + ([], false), // nothing + ] as [([(String, CGFloat)], Bool)]) + func recognizedInvariant(_ lines: [(String, CGFloat)], _ expected: Bool) { + let r = parse(lines) + #expect(r.recognized == expected) + // and the rule it is meant to encode still describes those cases + #expect(r.recognized == !(r.producer.isEmpty && r.vintage.isEmpty)) + } + + @Test("A default LabelReading, as read() returns for an unusable frame, is not recognized") + func defaultReading() { + let r = LabelReading() + #expect(!r.recognized) + #expect(r.producer.isEmpty && r.name.isEmpty && r.vintage.isEmpty) + #expect(r.region.isEmpty && r.grape.isEmpty) + } +} + +// MARK: - Robustness against untrusted camera input + +@Suite("LabelScanner · hostile and malformed input") +struct LabelScannerRobustnessTests { + + /// Everything here is text the camera could plausibly produce, whether by + /// pointing it at a wall of small print, a foreign-language label, or + /// something that is not a bottle at all. None of it may crash the parse + /// or run away: an index-out-of-range or an unbounded scan here is a + /// crash-on-photo bug in the field. + + @Test("An extremely long single line terminates without crashing") + func veryLongLine() { + let long = String(repeating: "CHÂTEAU MARGAUX GRAND VIN ", count: 800) // ~20k chars + var reading = LabelReading() + let seconds = elapsed { reading = parse([(long, 0.09), ("2019", 0.04)]) } + #expect(seconds < budget) + #expect(reading.vintage == "2019") + // Not merely non-empty: the long line is still tidied and assigned, + // so the producer starts with the text that was actually on it. + #expect(reading.producer.hasPrefix("Château Margaux Grand Vin")) + #expect(reading.producer.hasSuffix("Château Margaux Grand Vin")) // trailing space trimmed + } + + @Test("Thousands of lines terminate without crashing") + func thousandsOfLines() { + let many = (0..<1000).map { ("Line number \($0) of label text", CGFloat($0) / 1000) } + var reading = LabelReading() + let seconds = elapsed { reading = parse(many) } + #expect(seconds < budget) + #expect(reading.recognized) + // The tallest line wins the producer slot, whatever it happens to be. + #expect(reading.producer == "Line number 999 of label text") + } + + @Test("A very long run of repeated digits terminates and yields nothing") + func repeatedDigits() { + var nines = LabelReading() + var cycled = LabelReading() + let seconds = elapsed { + nines = parse([(String(repeating: "9", count: 20_000), 0.05)]) + cycled = parse([(String(repeating: "1234567890", count: 2_000), 0.05)]) + } + #expect(seconds < budget) + #expect(!nines.recognized) + #expect(!cycled.recognized) + #expect(nines.vintage == "") + #expect(cycled.vintage == "") + } + + @Test("Punctuation-only text is rejected rather than named a producer", arguments: [ + String(repeating: "!@#$%^&*()", count: 200), + "................", + "-----", + "'''''''", + "«»¿¡§¶†‡", + "\\\\//||", + ]) + func punctuationOnly(_ text: String) { + var reading = LabelReading() + let seconds = elapsed { reading = parse([(text, 0.05)]) } + #expect(seconds < budget) + #expect(reading.producer == "") + #expect(!reading.recognized) + } + + @Test("Degenerate unicode is handled without crashing", arguments: [ + "\u{0301}\u{0301}\u{0301}\u{0301}", // orphan combining marks + "\u{200B}\u{200D}\u{FEFF}", // zero-width and BOM + "\u{202E}gnitset", // right-to-left override + "\u{202A}\u{202B}\u{202C}\u{202D}", // unpaired bidi embeddings + "👨‍👩‍👧‍👦", // ZWJ grapheme cluster + "🍷🍷🍷🍷🍷", + "\u{FFFD}\u{FFFD}", // replacement characters + "a\u{0300}\u{0301}\u{0302}\u{0303}\u{0304}", // stacked diacritics + ]) + func degenerateUnicode(_ text: String) { + var reading = LabelReading() + let seconds = elapsed { reading = parse([(text, 0.05), ("Winery", 0.09)]) } + #expect(seconds < budget) + // The only contract is that it comes back at all, with the ordinary + // line still winning the producer slot. + #expect(reading.producer == "Winery") + } + + @Test("A combining-mark bomb terminates") + func combiningMarkBomb() { + let bomb = "A" + String(repeating: "\u{0301}", count: 5_000) + var reading = LabelReading() + let seconds = elapsed { reading = parse([(bomb, 0.05), ("Winery", 0.09)]) } + #expect(seconds < budget) + #expect(reading.producer == "Winery") + } + + @Test("A long run of ZWJ emoji clusters terminates") + func emojiClusters() { + let soup = String(repeating: "👨‍👩‍👧‍👦", count: 2_000) + var reading = LabelReading() + let seconds = elapsed { reading = parse([(soup, 0.05), ("Winery", 0.09)]) } + #expect(seconds < budget) + #expect(reading.producer == "Winery") + } + + @Test("Right-to-left and non-Latin scripts parse without crashing") + func rightToLeftScripts() { + let arabic = parse([("نبيذ أحمر", 0.06), ("Château", 0.09)]) + #expect(arabic.producer == "Château") + + let hebrew = parse([("יין אדום", 0.06), ("2019", 0.04)]) + #expect(hebrew.vintage == "2019") + + // Arabic-Indic digits are not ASCII digits and are not read as a year. + #expect(parse(withNeutralProducer: "٢٠١٩").vintage == "") + + let cjk = parse([("赤ワイン", 0.06), ("2019", 0.04)]) + #expect(cjk.vintage == "2019") + } + + /// The "Place, XX" shape only fires for a 3...30 character place. Asserting + /// the producer alone would pass whatever the region rule did with these, + /// so each case pins whether the line was taken as a region at all. + @Test("The 'Place, XX' shape fires only for a 3...30 character place", arguments: [ + ("A, DE", false), // 1 — below the floor + ("AB, DE", false), // 2 — below the floor + ("ABC, DE", true), // 3 — the floor itself + ("ABCDE, DE", true), + ("ABCDEF, DE", true), + ("ABCDEFGHIJKLMNOPQRSTUVWXYZABCD, DE", true), // 30 — the ceiling + ("ABCDEFGHIJKLMNOPQRSTUVWXYZABCDE, DE", false), // 31 — over it + ]) + func shapeLengthBoundaries(_ text: String, _ isRegion: Bool) { + var reading = LabelReading() + let seconds = elapsed { reading = parse([("Winery", 0.09), (text, 0.05)]) } + #expect(seconds < budget) + #expect(reading.producer == "Winery") + #expect(reading.region.hasSuffix(", DE") == isRegion) + // A line rejected by the shape is not consumed, so it stays in the + // ranking and becomes the cuvée instead. + #expect(reading.name.isEmpty == isRegion) + } + + /// `snapToKnownRegion` spends a tiered edit budget: nothing under six + /// characters, one edit at six to eight, two above. Only the long-name tier + /// is exercised elsewhere, so the two cheaper tiers are pinned here — a + /// widened budget is exactly the change that would start snapping unrelated + /// estates onto appellations. + @Test("The snapping budget is nothing under six characters, one edit at six", arguments: [ + ("Mosek, DE", "Mosek, DE"), // 5 chars, one edit from Mosel — no leeway + ("Baralo, IT", "Barolo, IT"), // 6 chars, one edit from Barolo — snaps + ("Barala, IT", "Barala, IT"), // 6 chars, two edits — over budget + ("Chiantt, IT", "Chianti, IT"), // 7 chars, one edit — snaps + ]) + func snapBudgetTiers(_ text: String, _ expected: String) { + #expect(parse([("Winery", 0.09), (text, 0.05)]).region == expected) + } + + @Test("Repeating a snap-eligible line many times terminates") + func manySnapCandidates() { + let lines = (0..<500).map { _ in ("Rieiniessen, DE", CGFloat(0.05)) } + var reading = LabelReading() + let seconds = elapsed { reading = parse(lines) } + #expect(seconds < budget) + #expect(reading.region == "Rheinhessen, DE") + } + + @Test("Zero and negative glyph heights do not break the ranking") + func degenerateHeights() { + // Equal heights leave the order to `sorted`, which is not stable, so + // the contract is that the two lines fill the two slots — not which + // one lands where. `!isEmpty` alone would also pass if both slots held + // the same line, or a slice of one. + let zero = parse([("Alpha", 0), ("Bravo", 0)]) + #expect(Set([zero.producer, zero.name]) == Set(["Alpha", "Bravo"])) + + let negative = parse([("Alpha", -1), ("Bravo", -2), ("2019", -3)]) + #expect(negative.producer == "Alpha") + #expect(negative.vintage == "2019") + + let extreme = parse([("Alpha", .greatestFiniteMagnitude), ("Bravo", -.greatestFiniteMagnitude)]) + #expect(extreme.producer == "Alpha") + #expect(extreme.name == "Bravo") + } + + @Test("Every line being identical terminates and picks one of them") + func identicalLines() { + let lines = (0..<200).map { _ in ("Château Margaux", CGFloat(0.05)) } + var reading = LabelReading() + let seconds = elapsed { reading = parse(lines) } + #expect(seconds < budget) + #expect(reading.producer == "Château Margaux") + #expect(reading.name == "Château Margaux") + } +} diff --git a/VinnotaTests/WineModelTests.swift b/VinnotaTests/WineModelTests.swift new file mode 100644 index 0000000..6f8663d --- /dev/null +++ b/VinnotaTests/WineModelTests.swift @@ -0,0 +1,1090 @@ +import Foundation +import SwiftData +import SwiftUI +import Testing +import UIKit + +@testable import Vinnota + +// MARK: - Fixtures +// +// Everything here builds a `Wine` through its real initialiser and then sets +// the stored properties the app's own commit paths set (`TastingView.markTasted`, +// `BoughtSheet.confirm`, `DetailView.set(_:toast:)`). Nothing re-implements a +// derived property — the point of these tests is to pin down what `Wine` and the +// two enums already decide, because those decisions are what the user sees. + +private func bottle( + producer: String = "Rinaldi", + name: String = "", + vintage: String = "", + region: String = "", + grape: String = "", + shop: String = "", + status: WineStatus = .new, + verdict: Verdict? = nil, + addedByHand: Bool = false +) -> Wine { + let wine = Wine(producer: producer, name: name, vintage: vintage, + region: region, grape: grape, shop: shop) + wine.status = status + wine.verdict = verdict + wine.addedByHand = addedByHand + return wine +} + +/// A fully filled bottle, so a search test can say which field it hit. +private func fullBottle() -> Wine { + bottle(producer: "Giuseppe Rinaldi", name: "Brunate", vintage: "2019", + region: "Barolo", grape: "Nebbiolo", shop: "Enoteca Sciolla") +} + +private func newContext() throws -> ModelContext { + let container = try ModelContainer( + for: Schema([Wine.self, TastingNote.self]), + configurations: [ModelConfiguration(isStoredInMemoryOnly: true)] + ) + return ModelContext(container) +} + +/// `Color` is `Equatable`, but two colours built by different expressions can +/// compare unequal even when they paint the same pixels. Resolving through +/// `UIColor` compares what the user actually sees, which is the property these +/// mapping tests care about. +private func rgba(_ color: Color) -> [CGFloat] { + let ui = UIColor(color) + var r: CGFloat = 0, g: CGFloat = 0, b: CGFloat = 0, a: CGFloat = 0 + if ui.getRed(&r, green: &g, blue: &b, alpha: &a) { + return [r, g, b, a].map { ($0 * 1000).rounded() / 1000 } + } + let sRGB = CGColorSpace(name: CGColorSpace.sRGB)! + let components = ui.cgColor.converted(to: sRGB, intent: .defaultIntent, options: nil)? + .components ?? [] + return components.map { ($0 * 1000).rounded() / 1000 } +} + +private func elapsed(_ body: () -> Void) -> TimeInterval { + let start = Date() + body() + return Date().timeIntervalSince(start) +} + +/// The real runs land around 0.05s, so 5s is a ~100× margin — generous enough +/// never to flake on a loaded machine, tight enough that a runaway scan fails +/// the suite instead of wedging it. +private let budget: TimeInterval = 5 + +/// Every `(status, addedByHand)` pair the chip has to render, hoisted out of the +/// test so `chipTableIsExhaustive` can check it really is every pair. Written as +/// literals rather than derived, so a change to the rule fails the test instead +/// of agreeing with itself. +private let chipCases: [(status: WineStatus, byHand: Bool, expected: String)] = [ + (.new, false, "Scanned"), + (.new, true, "Added by hand"), + (.want, false, "Want to try"), + (.want, true, "Want to try"), + (.maybe, false, "Undecided"), + (.maybe, true, "Undecided"), + (.not, false, "Passed"), + (.not, true, "Passed"), + (.bought, false, "In the rack"), + (.bought, true, "In the rack"), + (.tasted, false, "Tasted"), + (.tasted, true, "Tasted"), +] + +// MARK: - Which fields search reads + +@Suite("Wine.matches · fields searched") +struct SearchFieldTests { + + @Test("Producer, cuvée, region, grape and shop are all searchable", + arguments: [ + ("giuseppe rinaldi", "producer"), + ("brunate", "cuvée"), + ("barolo", "region"), + ("nebbiolo", "grape"), + ("enoteca sciolla", "shop"), + ]) + func everySearchedField(query: String, field: String) { + #expect(fullBottle().matches(query: query), "\(field) should be searchable") + } + + /// The placeholder promises "Producer, region, grape". The row that comes + /// back leads with the vintage in a 42pt column, so a user reading the + /// results has every reason to think the year is searchable too. + /// + /// FINDING: Vinnota/Model/Wine.swift:162 omits `vintage` from the searched + /// fields. Typing the year of a bottle you can see in the list returns + /// "No bottle matches that." + @Test("FINDING: the vintage is not searchable") + func vintageIsNotSearched() { + let wine = fullBottle() + #expect(wine.displayVintage == "2019") + #expect(wine.matches(query: "2019") == false, "documented current behaviour") + } + + /// Notes are a separate `@Model`; `matches` never touches the relationship. + /// FINDING: Vinnota/Model/Wine.swift:162 — a bottle whose only mention of + /// "cork taint" is in a tasting note cannot be found by searching for it. + @Test("FINDING: tasting notes are not searchable") + func notesAreNotSearched() { + let wine = fullBottle() + let note = TastingNote(kind: .text, phase: .post, text: "unmistakable cork taint") + note.wine = wine + wine.notes = [note] + + #expect(wine.notes.first?.text.contains("cork taint") == true) + #expect(wine.matches(query: "cork taint") == false, "documented current behaviour") + } + + /// Price and dates are not searched either — asserted so that adding them + /// later is a deliberate change with a failing test to update. + @Test(arguments: ["45", "€45", "05 Sep 2026"]) + func pricesAndDatesAreNotSearched(query: String) { + let wine = fullBottle() + wine.price = "45" + wine.boughtPrice = "45" + wine.boughtDate = "05 Sep 2026" + #expect(wine.matches(query: query) == false) + } + + @Test("Matching is case-insensitive in both directions", + arguments: ["BAROLO", "BaRoLo"]) + func caseIsIgnored(query: String) { + #expect(fullBottle().matches(query: query)) + #expect(bottle(producer: "BAROLO").matches(query: query)) + } + + @Test("A partial run of characters matches, anywhere in the word", + arguments: [ + ("gius", true), // start of a word + ("aldi", true), // end of a word + ("ebbio", true), // buried in the middle + ("o", true), // a single letter + ("naldi bru", true), // across a word boundary + ("innal", false), // letters that are all present but not in order + ("ldia", false), + ]) + func partialMatches(query: String, expected: Bool) { + #expect(fullBottle().matches(query: query) == expected) + } + + @Test("A query that is nowhere in any field does not match") + func missMatchesNothing() { + #expect(fullBottle().matches(query: "Riesling") == false) + #expect(fullBottle().matches(query: "giuseppe rinaldix") == false) + } +} + +// MARK: - The empty query + +@Suite("Wine.matches · the empty query means no filter") +struct EmptyQueryTests { + + /// Space, tab and the Unicode spaces a paste can carry are all trimmed by + /// `.whitespaces`, so each of these shows the whole cellar. + @Test(arguments: [ + "", + " \t \t ", + "\u{00A0}", // NO-BREAK SPACE + "\u{2009}", // THIN SPACE + "\u{3000}", // IDEOGRAPHIC SPACE + ]) + func blankQueriesMatchEveryBottle(query: String) { + #expect(fullBottle().matches(query: query)) + #expect(bottle(producer: "", name: "", region: "", grape: "", shop: "") + .matches(query: query), + "even a bottle with nothing in any searched field") + } + + @Test("Padding around a real query is trimmed away") + func paddingIsTrimmed() { + #expect(fullBottle().matches(query: " barolo ")) + #expect(fullBottle().matches(query: "\tbarolo\t")) + } + + /// `matches` trims `.whitespaces`, which does not include newlines, while + /// every other blank check in the app uses `.whitespacesAndNewlines`. The + /// newline-*only* query is already pinned down in FormValidationTests; the + /// case that bites a real user is a **paste**, which routinely carries a + /// trailing newline and turns a good query into a guaranteed miss. + /// + /// FINDING: Vinnota/Model/Wine.swift:160 — use `.whitespacesAndNewlines`. + @Test("FINDING: a pasted query with a newline on it finds nothing") + func newlinePaddingIsNotTrimmed() { + let wine = fullBottle() + #expect(wine.matches(query: "barolo")) + #expect(wine.matches(query: "barolo\n") == false, "documented current behaviour") + #expect(wine.matches(query: "\nbarolo") == false, "documented current behaviour") + #expect(wine.matches(query: "barolo\r\n") == false, "documented current behaviour") + } + + /// A zero-width space is not whitespace, so it survives the trim — but the + /// comparison treats it as ignorable, so it still behaves as "no filter" + /// rather than hiding the cellar. Benign, and worth knowing it is benign. + @Test("A zero-width space query is harmless") + func zeroWidthSpaceQuery() { + #expect(fullBottle().matches(query: "\u{200B}")) + } +} + +// MARK: - Diacritics + +@Suite("Wine.matches · accents") +struct DiacriticTests { + + /// The whole domain is accented — Château, Rhône, Pétrus, Gevrey-Chambertin + /// — and an iOS keyboard makes an accented character deliberate work. The + /// search field even disables autocorrect and autocapitalisation, so nothing + /// is going to put the circumflex there for the user. + /// + /// FINDING: Vinnota/Model/Wine.swift:160,163 lowercases but does not fold + /// diacritics, so the unaccented spelling — which is what most users type — + /// matches nothing. `localizedStandardContains` (or `range(of:options: + /// [.caseInsensitive, .diacriticInsensitive])`) would fix it. + @Test("FINDING: an unaccented query does not find an accented bottle", + arguments: [ + ("Château Margaux", "chateau"), + ("Côtes du Rhône", "rhone"), + ("Pétrus", "petrus"), + ("Gevrey-Chambertin Clos Saint-Jacques", "clos saint-jacques"), + ]) + func unaccentedQueryMisses(field: String, query: String) { + let wine = bottle(producer: field) + // The last row is the control: no accent in the field, so it matches. + let hasAccent = field.folding(options: .diacriticInsensitive, locale: nil) != field + #expect(wine.matches(query: query) == !hasAccent, "documented current behaviour") + } + + @Test("Typing the accent does work", + arguments: [ + ("Château Margaux", "château"), + ("Château Margaux", "CHÂTEAU"), + ("Côtes du Rhône", "rhône"), + ("Pétrus", "pétrus"), + ]) + func accentedQueryHits(field: String, query: String) { + #expect(bottle(producer: field).matches(query: query)) + } + + /// The mirror image: an accented query against an unaccented bottle also + /// misses, so an OCR read that dropped the accent is unfindable by the + /// correct spelling. + @Test("FINDING: an accented query does not find an unaccented bottle") + func accentedQueryMissesPlainBottle() { + #expect(bottle(producer: "Chateau Margaux").matches(query: "château") == false, + "documented current behaviour") + } + + /// Composed vs decomposed is the one normalisation question that *does* go + /// the right way: Swift's comparison is canonically equivalent, so a bottle + /// scanned as U+0065 U+0301 is found by a query typed as U+00E9 and back. + @Test("Composed and decomposed accents are interchangeable") + func canonicalEquivalence() { + let composed = "Ch\u{00E2}teau P\u{00E9}trus" // â, é + let decomposed = "Cha\u{0302}teau Pe\u{0301}trus" // a + ̂ , e + ́ + #expect(Array(composed.unicodeScalars) != Array(decomposed.unicodeScalars), + "genuinely different bytes") + #expect(composed == decomposed, + "…which Swift's own `==` already treats as the same string") + + #expect(bottle(producer: composed).matches(query: decomposed)) + #expect(bottle(producer: decomposed).matches(query: composed)) + #expect(bottle(producer: decomposed).matches(query: "p\u{00E9}trus")) + #expect(bottle(producer: composed).matches(query: "pe\u{0301}trus")) + } + + /// German sharp s does not fold to "ss" here either — same root cause, + /// recorded so the fix above is judged against it. + @Test("FINDING: ß is not folded to ss") + func sharpSIsNotFolded() { + #expect(bottle(producer: "Weingut Straße").matches(query: "strasse") == false, + "documented current behaviour") + #expect(bottle(producer: "Weingut Straße").matches(query: "straße")) + } +} + +// MARK: - The joined-fields seam + +@Suite("Wine.matches · fields are joined before the search") +struct JoinedFieldTests { + + /// `matches` joins the five fields with a space and searches the result, so + /// a query can straddle two fields. "brunate barolo" is not in any single + /// field, but it matches — which is usually what a user wanted anyway. + @Test("A query can span two adjacent fields") + func queryCanSpanFields() { + #expect(fullBottle().matches(query: "brunate barolo")) + #expect(fullBottle().matches(query: "rinaldi brunate")) + } + + /// The other half of the same behaviour: an *empty* field still contributes + /// its separator, so two spaces appear where it was and a natural query + /// across the gap misses. + /// + /// FINDING: Vinnota/Model/Wine.swift:163 joins unconditionally. Filtering + /// blank fields out before joining — as `subtitle` and `eyebrow` already do + /// — would make this consistent. + @Test("FINDING: a blank field leaves a double space that breaks a spanning query") + func blankFieldBreaksASpanningQuery() { + let noCuvee = bottle(producer: "Rinaldi", name: "", region: "Barolo") + #expect(noCuvee.matches(query: "rinaldi barolo") == false, + "documented current behaviour") + #expect(noCuvee.matches(query: "rinaldi barolo"), "two spaces do match") + } + + /// Search never looks past the searched fields into the ones next door. + @Test("Each field is matched on its own content") + func fieldsAreNotConfused() { + let wine = bottle(producer: "Rinaldi", region: "Barolo", shop: "Enoteca") + #expect(wine.matches(query: "enoteca")) + #expect(wine.matches(query: "vinoteca") == false) + } +} + +// MARK: - Untrusted query text + +@Suite("Wine.matches · untrusted input is literal text") +struct SearchRobustnessTests { + + /// The query goes straight into `String.contains`, which is a literal + /// substring search. If it were ever swapped for `NSPredicate`, `LIKE` or a + /// regex, these would start throwing or matching everything. One + /// representative per family rather than a long list of near-twins: an + /// unbalanced group that would fail to compile, a character class, anchors, + /// an escape, a catastrophic-backtracking pattern, the two SQL wildcards, + /// and a format specifier. + @Test("Regex metacharacters are searched literally, never compiled", + arguments: [ + ".*", "(((", "[a-z]+", "^Rinaldi$", "\\d{4}", "a|b", + "\\", "(?i)rinaldi", "(a+)+$", "%", "_", "%@", "'", + ]) + func metacharactersAreLiteral(query: String) { + let plain = bottle(producer: "Giuseppe Rinaldi", region: "Barolo") + #expect(plain.matches(query: query) == false, + "a pattern must not match a bottle that does not contain it as text") + + // …and the same characters *are* found when they are really in the data. + let literal = bottle(producer: "Giuseppe Rinaldi \(query) Barolo") + #expect(literal.matches(query: query)) + } + + /// The cases that separate "literal" from "pattern" most clearly: a + /// single-character wildcard must not match the character it stands in for. + /// `.` is the regex one, `_` the SQL one — `matches` runs in Swift, not in + /// the store, so nothing ever reaches a `LIKE` either. + @Test("A single-character wildcard matches only itself") + func wildcardsAreNotWildcards() { + #expect(bottle(producer: "Dom. Rinaldi").matches(query: "m. r")) + #expect(bottle(producer: "Dom Rinaldi").matches(query: "m. r") == false) + #expect(bottle(producer: "Rinaldi").matches(query: ".") == false) + #expect(bottle(producer: "Rinaldi").matches(query: "R_naldi") == false) + #expect(bottle(producer: "100% Nebbiolo").matches(query: "100%")) + } + + @Test("Emoji, RTL text and control characters are ordinary query text", + arguments: [ + "🍇🍷", + "\u{202E}gnitset", // RIGHT-TO-LEFT OVERRIDE + "\u{200F}מרגו\u{200E}", // RTL/LTR marks around Hebrew + "\u{0000}", // NUL + "\u{1F1EB}\u{1F1F7}", // flag, a two-scalar grapheme + "e\u{0301}\u{0323}", // stacked combining marks + ]) + func exoticTextIsJustText(query: String) { + #expect(bottle(producer: "Château Margaux \(query)").matches(query: query)) + #expect(bottle(producer: "Château Margaux").matches(query: query) == false) + } + + @Test("An RTL producer is findable by an RTL substring") + func rtlSubstring() { + #expect(bottle(producer: "شاتو مارغو", region: "بوردو").matches(query: "مارغو")) + #expect(bottle(producer: "שאטו מרגו").matches(query: "מרגו")) + } + + @Test("A very long query returns quickly and does not crash") + func veryLongQuery() { + let wine = fullBottle() + let long = String(repeating: "a", count: 200_000) + let emoji = String(repeating: "🍷", count: 50_000) + + let took = elapsed { + #expect(wine.matches(query: long) == false) + #expect(wine.matches(query: emoji) == false) + #expect(wine.matches(query: String(repeating: " ", count: 100_000)), + "100k spaces is still an empty query") + } + #expect(took < budget) + } + + @Test("A very long field does not blow up the search") + func veryLongField() { + let wine = bottle(producer: String(repeating: "Château Rinaldi ", count: 20_000)) + let took = elapsed { + #expect(wine.matches(query: "château rinaldi")) + #expect(wine.matches(query: "riesling") == false) + } + #expect(took < budget) + } + + /// Searching must never mutate the bottle it is filtering. + @Test("matches has no side effects") + func noSideEffects() { + let wine = fullBottle() + wine.status = .bought + _ = wine.matches(query: "barolo") + _ = wine.matches(query: ".*") + _ = wine.matches(query: "") + #expect(wine.producer == "Giuseppe Rinaldi") + #expect(wine.status == .bought) + #expect(wine.vintage == "2019") + } +} + +// MARK: - Status chip copy + +@Suite("Wine.statusLabel") +struct StatusLabelTests { + + /// `addedByHand` only ever changes the `.new` chip: once a bottle has been + /// ranked, bought or drunk, its own status is the truer label. + @Test(arguments: chipCases) + func chipCopy(row: (status: WineStatus, byHand: Bool, expected: String)) { + #expect(bottle(status: row.status, addedByHand: row.byHand).statusLabel + == row.expected) + } + + /// The table above is only as good as its coverage, so check the coverage + /// rather than merely counting the enum: every status must appear with + /// `addedByHand` both ways, or a new status could ship with no pinned chip. + @Test("Every status is in the chip table, both ways") + func chipTableIsExhaustive() { + for status in WineStatus.allCases { + let rows = chipCases.filter { $0.status == status } + #expect(Set(rows.map(\.byHand)) == [true, false], + "\(status.rawValue) is missing a row") + } + #expect(chipCases.count == WineStatus.allCases.count * 2, + "and no status is pinned twice over") + } + + /// The unqualified enum label still says "Scanned" for `.new`; the untruth + /// is corrected by `Wine.statusLabel`, not by the enum, so a view that + /// reaches for `wine.status.label` would reintroduce the bug. + @Test("The bare enum label is the un-corrected copy") + func enumLabelIsUncorrected() { + #expect(WineStatus.new.label == "Scanned") + #expect(bottle(status: .new, addedByHand: true).status.label == "Scanned") + #expect(bottle(status: .new, addedByHand: true).statusLabel == "Added by hand") + } + + @Test("Default bottles are `.new` and not marked hand-entered") + func defaults() { + let wine = Wine(producer: "Rinaldi", name: "", vintage: "", region: "", + grape: "", shop: "") + #expect(wine.status == .new) + #expect(wine.addedByHand == false) + #expect(wine.statusLabel == "Scanned") + #expect(wine.verdict == nil) + #expect(wine.qty == 1) + #expect(wine.openedAt == nil) + } + + /// A corrupt or future `statusRaw` falls back to `.new` rather than + /// trapping — the store is the only thing that can produce this. + @Test func unknownStoredStatusFallsBackToNew() { + let wine = bottle() + wine.statusRaw = "cellared" + #expect(wine.status == .new) + #expect(wine.statusLabel == "Scanned") + + wine.verdictRaw = "ecstatic" + #expect(wine.verdict == nil) + } +} + +// MARK: - Display strings + +@Suite("Wine display strings") +struct DisplayStringTests { + + /// Older rows stored an em dash to mean "no cuvée"; `displayName` turns + /// that back into nothing so the card does not print a stray dash. + @Test("The em-dash sentinel and blanks both display as nothing", + arguments: ["—", "", " ", " ", "\t", "\n", "\u{00A0}"]) + func emptyCuvees(name: String) { + #expect(bottle(name: name).displayName == "") + #expect(bottle(name: name, region: "Barolo").subtitle == "Barolo", + "no leading separator") + } + + @Test("A real cuvée is passed through untouched", + arguments: ["Brunate", "Clos du —", "——", "Côte-Rôtie", "-", "– ", "N.V."]) + func realCuvees(name: String) { + #expect(bottle(name: name).displayName == name) + } + + /// The sentinel is compared without trimming, so a padded em dash — which + /// only a legacy row can hold, since the commit path trims — still prints. + /// FINDING: Vinnota/Model/Wine.swift:127 compares `name == "—"` before + /// `isBlank`; `name.trimmed == "—"` would cover the legacy row too. + @Test("FINDING: a padded em dash is not recognised as the sentinel") + func paddedSentinel() { + #expect(bottle(name: " — ").displayName == " — ", "documented current behaviour") + #expect(bottle(name: " — ", region: "Barolo").subtitle == " — · Barolo", + "documented current behaviour") + } + + @Test("subtitle joins the cuvée and the region with a middle dot") + func subtitle() { + #expect(bottle(name: "Brunate", region: "Barolo").subtitle == "Brunate · Barolo") + #expect(bottle(name: "Brunate", region: "").subtitle == "Brunate") + #expect(bottle(name: "", region: "Barolo").subtitle == "Barolo") + #expect(bottle(name: "", region: "").subtitle == "", + "the card hides the line entirely") + #expect(bottle(name: "—", region: "").subtitle == "") + } + + @Test("eyebrow joins the grape and the region, in that order") + func eyebrow() { + #expect(bottle(region: "Barolo", grape: "Nebbiolo").eyebrow == "Nebbiolo · Barolo") + #expect(bottle(region: "", grape: "Nebbiolo").eyebrow == "Nebbiolo") + #expect(bottle(region: "Barolo", grape: "").eyebrow == "Barolo") + #expect(bottle(region: "", grape: "").eyebrow == "") + } + + /// `eyebrow` uses the raw name, not `displayName`, so it differs from + /// `subtitle` in what it treats as empty — but it never reads the cuvée, so + /// the em-dash sentinel cannot leak into it. + @Test("eyebrow never shows the cuvée") + func eyebrowIgnoresCuvee() { + #expect(bottle(name: "—", region: "Barolo", grape: "Nebbiolo").eyebrow + == "Nebbiolo · Barolo") + } + + /// A whitespace-only region is not empty, so it does survive into both + /// joins as a stray separator. The commit path trims, so this is reachable + /// only from a legacy row. + @Test("FINDING: a whitespace-only region still contributes a separator") + func whitespaceRegion() { + #expect(bottle(name: "Brunate", region: " ").subtitle == "Brunate · ", + "documented current behaviour") + #expect(bottle(region: " ", grape: "Nebbiolo").eyebrow == "Nebbiolo · ", + "documented current behaviour") + } + + @Test("hasVintage is false for a blank year, true for anything else", + arguments: ["", " ", " ", "\t", "\n", "\r\n", "\u{00A0}", "\u{3000}"]) + func blankVintages(vintage: String) { + let wine = bottle(vintage: vintage) + #expect(wine.hasVintage == false, "the card falls back to NV") + #expect(wine.displayVintage == vintage, "displayVintage is the raw value") + } + + @Test(arguments: ["2019", "NV", "1985", "20 19", "MMXIX", "0"]) + func realVintages(vintage: String) { + let wine = bottle(vintage: vintage) + #expect(wine.hasVintage) + #expect(wine.displayVintage == vintage) + } + + /// `hasVintage` trims but `displayVintage` does not, so a padded year is + /// shown padded. Again only reachable from a legacy row. + @Test("FINDING: displayVintage does not trim what hasVintage trimmed") + func paddedVintage() { + let wine = bottle(vintage: " 2019 ") + #expect(wine.hasVintage) + #expect(wine.displayVintage == " 2019 ", "documented current behaviour") + } + + /// The display fields never touch the underlying store values. + @Test func displayIsNonDestructive() { + let wine = bottle(name: "—", vintage: " 2019 ", region: "Barolo") + _ = (wine.displayName, wine.subtitle, wine.eyebrow, wine.displayVintage, wine.hasVintage) + #expect(wine.name == "—") + #expect(wine.vintage == " 2019 ") + } +} + +// MARK: - Money +// +// `displayPrice` and `CurrencyCode.format` were the one part of the two files +// under test that no suite reached: FormValidationTests pins how a price is +// *stored* (`hasPrice`), but nothing pinned how it is *rendered*. + +@Suite("Wine.displayPrice and CurrencyCode.format") +struct PriceTests { + + /// The two currencies that are not "symbol then amount": SEK trails, and + /// CHF carries a thin space rather than the bare `symbol`. + @Test(arguments: [ + (CurrencyCode.EUR, "45", "€45"), + (CurrencyCode.USD, "45", "$45"), + (CurrencyCode.GBP, "45", "£45"), + (CurrencyCode.CHF, "45", "Fr\u{2009}45"), + (CurrencyCode.SEK, "45", "45 kr"), + ]) + func formatting(currency: CurrencyCode, amount: String, expected: String) { + #expect(currency.format(amount) == expected) + } + + @Test("CHF's thin space is not the plain symbol") + func chfIsNotJustItsSymbol() { + #expect(CurrencyCode.CHF.symbol == "Fr") + #expect(CurrencyCode.CHF.format("45") != "Fr45") + #expect(CurrencyCode.CHF.format("45").contains("\u{2009}")) + } + + @Test("A missing amount is an em dash, never a bare symbol", + arguments: CurrencyCode.allCases) + func noAmount(currency: CurrencyCode) { + #expect(currency.format(nil) == "—") + #expect(currency.format("") == "—") + } + + /// The paid price wins when there is one, and it is rendered in the *paid* + /// currency — the two are independent fields, so a bottle marked in EUR and + /// bought in SEK must not print the shelf currency. + @Test func paidPriceWinsAndCarriesItsOwnCurrency() { + let wine = bottle() + wine.price = "45" + wine.currency = .EUR + #expect(wine.displayPrice == "€45", "no paid price yet, so the shelf price shows") + #expect(wine.hasPrice) + + wine.boughtPrice = "380" + wine.boughtCurrency = .SEK + #expect(wine.displayPrice == "380 kr") + #expect(wine.hasPrice) + } + + @Test("With neither price the line is an em dash") + func noPriceAtAll() { + let wine = bottle() + #expect(wine.price == nil) + #expect(wine.boughtPrice == nil) + #expect(wine.hasPrice == false) + #expect(wine.displayPrice == "—") + } + + /// `Wine.init` seeds `boughtCurrency` from the shelf currency, so a bottle + /// bought without touching the currency picker renders in the right one. + @Test func boughtCurrencyDefaultsToTheShelfCurrency() { + let wine = Wine(producer: "Rinaldi", name: "", vintage: "", region: "", + grape: "", shop: "", price: "45", currency: .GBP) + #expect(wine.boughtCurrency == .GBP) + wine.boughtPrice = "40" + #expect(wine.displayPrice == "£40") + } + + /// `BoughtSheet.confirm` guards with `isEmpty`, not `isBlank`, so a single + /// space typed into the paid-price field is stored verbatim. `format` only + /// rejects `isEmpty` too, so it prints the currency symbol against nothing. + /// + /// FINDING: Vinnota/Views/Sheets/BoughtSheet.swift:86 and + /// Vinnota/Model/Enums.swift:130 — both should test `isBlank`. + @Test("FINDING: a whitespace-only paid price prints a bare currency symbol") + func whitespacePaidPrice() { + let wine = bottle() + wine.price = "45" + wine.boughtPrice = " " + #expect(wine.hasPrice, "documented current behaviour") + #expect(wine.displayPrice == "€ ", "documented current behaviour") + } + + /// The same seam one step further on: an *empty* paid price is only + /// reachable from a legacy row, and it hides a perfectly good shelf price. + @Test("FINDING: an empty paid price hides the shelf price behind an em dash") + func emptyPaidPriceHidesShelfPrice() { + let wine = bottle() + wine.price = "45" + wine.boughtPrice = "" + #expect(wine.currency.format(wine.price) == "€45", "the shelf price is still there") + #expect(wine.displayPrice == "—", "documented current behaviour") + #expect(wine.hasPrice == false) + } +} + +// MARK: - Notes + +@Suite("Wine.preNotes and Wine.postNotes") +struct NoteSplitTests { + + /// `createdAt` is set explicitly: two notes built in the same instant get + /// equal timestamps, and `sorted(by:)` is not stable, so a test that relied + /// on insertion order would flake rather than pin the sort. + private func note(_ phase: TastingNote.Phase, _ text: String, + at offset: TimeInterval) -> TastingNote { + let note = TastingNote(kind: .text, phase: phase, text: text) + note.createdAt = Date(timeIntervalSince1970: offset) + return note + } + + @Test("Each phase gets only its own notes, oldest first") + func splitAndOrder() { + let wine = bottle() + // Deliberately attached newest-first, so the sort has work to do. + wine.notes = [ + note(.post, "second in the glass", at: 400), + note(.pre, "second on the shelf", at: 200), + note(.post, "first in the glass", at: 300), + note(.pre, "first on the shelf", at: 100), + ] + + #expect(wine.preNotes.map(\.text) == ["first on the shelf", "second on the shelf"]) + #expect(wine.postNotes.map(\.text) == ["first in the glass", "second in the glass"]) + } + + @Test("A bottle with no notes has two empty lists") + func noNotes() { + let wine = bottle() + #expect(wine.preNotes.isEmpty) + #expect(wine.postNotes.isEmpty) + } + + /// The detail screen's note byline: dictated notes say so, typed ones do not. + @Test(arguments: [ + (TastingNote.Kind.voice, "Dictated · 12 Aug · 18:40"), + (TastingNote.Kind.text, "Typed · 12 Aug · 18:40"), + ]) + func noteLabel(kind: TastingNote.Kind, expected: String) { + let note = TastingNote(kind: kind, phase: .pre, text: "tar and roses", + when: "12 Aug · 18:40") + #expect(note.label == expected) + } + + /// A corrupt or future `kindRaw`/`phaseRaw` falls back rather than trapping, + /// the same way `status` does. + @Test func unknownStoredKindAndPhaseFallBack() { + let note = TastingNote(kind: .voice, phase: .post, text: "x") + note.kindRaw = "telepathic" + note.phaseRaw = "during" + #expect(note.kind == .text) + #expect(note.phase == .pre) + } +} + +// MARK: - Deletion guard + +@Suite("Wine.canDelete · the data-loss guard") +struct DeletionGuardTests { + + /// The boundary, exactly: `.tasted` is the only status that locks a bottle + /// down. Written as literals so the rule cannot agree with a broken + /// implementation. + @Test(arguments: [ + (WineStatus.new, true), + (WineStatus.want, true), + (WineStatus.maybe, true), + (WineStatus.not, true), + (WineStatus.bought, true), + (WineStatus.tasted, false), + ]) + func onlyTastedBottlesAreProtected(status: WineStatus, deletable: Bool) { + let wine = bottle(status: status) + #expect(wine.canDelete == deletable) + #expect(wine.isTasted == (status == .tasted)) + #expect(wine.canDelete == !wine.isTasted, "the guard is exactly `not tasted`") + } + + @Test("isBought is exactly `.bought`, and is not implied by `.tasted`") + func isBought() { + #expect(bottle(status: .bought).isBought) + #expect(bottle(status: .tasted).isBought == false, + "a drunk bottle is no longer in the rack") + for s in WineStatus.allCases where s != .bought { + #expect(bottle(status: s).isBought == false) + } + } + + /// The keenness control is hidden once the decision has been made for you. + @Test(arguments: [ + (WineStatus.new, true), + (WineStatus.want, true), + (WineStatus.maybe, true), + (WineStatus.not, true), + (WineStatus.bought, false), + (WineStatus.tasted, false), + ]) + func showKeen(status: WineStatus, shown: Bool) { + #expect(bottle(status: status).showKeen == shown) + } + + /// The guard is keyed on status alone. A bottle carrying a verdict and a + /// cellar full of post-pour notes is still deletable if its status was + /// never moved to `.tasted` — which no commit path in the app produces + /// today, but nothing prevents either. + @Test("The guard reads status, not evidence of having been drunk") + func guardIsStatusOnly() { + let wine = bottle(status: .bought, verdict: .loved) + let note = TastingNote(kind: .text, phase: .post, text: "drunk and loved") + note.wine = wine + wine.notes = [note] + wine.openedAt = Formatters.today() + + #expect(wine.postNotes.count == 1) + #expect(wine.verdict == .loved) + #expect(wine.canDelete, "documented current behaviour") + } + + /// FINDING: Vinnota/Views/Sheets/DeleteDialog.swift:56 — `confirm()` calls + /// `context.delete(wine)` without consulting `canDelete`. The guard exists + /// only as a hidden trash icon in Vinnota/Views/DetailView.swift:104, so a + /// tasted bottle is protected by the layout and by nothing else. This test + /// documents that the model does not enforce it; the fix is a `guard + /// wine.canDelete else { return }` in `confirm()`. + @Test("FINDING: nothing below the view enforces canDelete") + func modelDoesNotEnforceTheGuard() throws { + let context = try newContext() + let tasted = bottle(status: .tasted, verdict: .loved) + context.insert(tasted) + try context.save() + #expect(tasted.canDelete == false) + + context.delete(tasted) + try context.save() + #expect(try context.fetch(FetchDescriptor()).isEmpty, + "documented current behaviour: the store deletes it anyway") + } + + /// When a deletable bottle does go, its notes go with it — the dialog + /// promises "every note on it go for good", and the cascade rule delivers. + @Test func deletingABottleTakesItsNotes() throws { + let context = try newContext() + let wine = bottle(status: .want) + context.insert(wine) + for text in ["smells of tar", "and of roses"] { + let note = TastingNote(kind: .text, phase: .pre, text: text) + note.wine = wine + context.insert(note) + } + try context.save() + #expect(try context.fetch(FetchDescriptor()).count == 2) + + #expect(wine.canDelete) + context.delete(wine) + try context.save() + + #expect(try context.fetch(FetchDescriptor()).isEmpty) + #expect(try context.fetch(FetchDescriptor()).isEmpty, + "cascade, so no orphaned notes are left behind") + } +} + +// MARK: - Lifecycle + +@Suite("The bottle lifecycle") +struct LifecycleTests { + + /// new → want → bought → tasted, driven exactly as the three commit paths + /// drive it (`DetailView.set`, `BoughtSheet.confirm`, `TastingView.markTasted`), + /// checking the user-visible consequences at each step. + @Test func theHappyPath() { + let wine = bottle(producer: "Rinaldi", status: .new, addedByHand: true) + #expect(wine.statusLabel == "Added by hand") + #expect(wine.showKeen) + #expect(wine.canDelete) + #expect(wine.verdict == nil) + + wine.status = .want + #expect(wine.statusLabel == "Want to try") + #expect(wine.showKeen) + #expect(wine.canDelete) + + wine.status = .bought + wine.boughtPrice = "42" + wine.qty = 6 + #expect(wine.statusLabel == "In the rack") + #expect(wine.isBought) + #expect(wine.showKeen == false, "the keenness control is gone") + #expect(wine.canDelete, "still deletable — nothing has been drunk") + + wine.status = .tasted + wine.verdict = .loved + wine.openedAt = Formatters.today() + #expect(wine.statusLabel == "Tasted") + #expect(wine.isTasted) + #expect(wine.isBought == false) + #expect(wine.showKeen == false) + #expect(wine.canDelete == false, "the record is now permanent") + } + + /// Marking tasted is one-way in the UI, but the model itself will happily + /// go back — and doing so hands the trash icon back. + @Test("FINDING: the model does not make `.tasted` terminal") + func tastedIsNotTerminalInTheModel() { + let wine = bottle(status: .tasted, verdict: .disliked) + #expect(wine.canDelete == false) + wine.status = .bought + #expect(wine.canDelete, "documented current behaviour") + #expect(wine.verdict == .disliked, "the verdict survives the walk-back") + #expect(wine.statusLabel == "In the rack") + } + + @Test("Verdict copy") + func verdictCopy() { + #expect(Verdict.loved.label == "Loved it") + #expect(Verdict.meh.label == "Fine") + #expect(Verdict.disliked.label == "Not for me") + #expect(Verdict.loved.caption == "Buy it again") + #expect(Verdict.meh.caption == "No hurry to repeat") + #expect(Verdict.disliked.caption == "Note it and move on") + #expect(Verdict.allCases.count == 3) + } + + @Test("Verdict dots are the semantic traffic-light colours") + func verdictDots() { + #expect(rgba(Verdict.loved.dot) == rgba(Palette.green)) + #expect(rgba(Verdict.meh.dot) == rgba(Palette.yellow)) + #expect(rgba(Verdict.disliked.dot) == rgba(Palette.red)) + #expect(rgba(Verdict.loved.dot) != rgba(Verdict.disliked.dot)) + } + + /// Only `.bought` and `.tasted` get a colour of their own; the three + /// keenness states are separated by opacity on the same rose, and `.new` + /// and `.maybe` share theirs exactly. + @Test func statusRails() { + #expect(rgba(WineStatus.bought.rail) == rgba(Palette.green)) + #expect(rgba(WineStatus.want.rail) == rgba(Palette.rose(1.0))) + #expect(rgba(WineStatus.tasted.rail) == rgba(Palette.rose(0.55))) + #expect(rgba(WineStatus.new.rail) == rgba(Palette.rose(0.22))) + #expect(rgba(WineStatus.not.rail) == rgba(Palette.rose(0.10))) + + // FINDING (cosmetic): Vinnota/Model/Enums.swift:23,25 — `.new` and + // `.maybe` are both rose(0.22), so the dot on a card cannot tell a + // freshly scanned bottle from one you deliberately shrugged at. + #expect(rgba(WineStatus.maybe.rail) == rgba(WineStatus.new.rail), + "documented current behaviour") + } + + /// The accent drawn on the card dot, the search-row rail and the timeline: + /// once there is a verdict it wins, whatever the status says. + @Test func verdictAccentBeatsStatusAccent() { + let untasted = bottle(status: .bought) + #expect(rgba(untasted.accent) == rgba(WineStatus.bought.rail)) + + let loved = bottle(status: .tasted, verdict: .loved) + #expect(rgba(loved.accent) == rgba(Verdict.loved.dot)) + #expect(rgba(loved.accent) != rgba(WineStatus.tasted.rail)) + + let disliked = bottle(status: .tasted, verdict: .disliked) + #expect(rgba(disliked.accent) == rgba(Verdict.disliked.dot)) + + // A verdict can be set on a bottle that was never marked tasted, and + // it takes the accent over even then. + let odd = bottle(status: .new, verdict: .meh) + #expect(rgba(odd.accent) == rgba(Verdict.meh.dot)) + #expect(odd.statusLabel == "Scanned", "the chip still reads from status") + } +} + +// MARK: - Filter tabs +// +// The cellar's *stat counts* are not reachable from a test: `CellarView.stats` +// is a `private var` inside the `View` and counts the `@Query` array inline +// (`wines.count`, `wines.filter { $0.status != .tasted }.count`, +// `wines.filter { $0.verdict == .loved }.count`, Vinnota/Views/CellarView.swift +// :54-57). There is no seam, and re-implementing those three expressions here +// would test a copy rather than the app, so they are deliberately left untested +// — extracting them onto `Wine`/`Array` is what would make them testable. +// +// The filter *predicate* is reachable: `WineFilter.matches(_:)` in +// Vinnota/Model/Enums.swift is what both CellarView and SearchView call, so the +// tests below drive the real thing. + +@Suite("WineFilter tabs") +struct FilterTests { + + @Test("The All tab shows every bottle, whatever its status", + arguments: WineStatus.allCases) + func allShowsEverything(status: WineStatus) { + #expect(WineFilter.all.matches(bottle(status: status))) + } + + @Test("Each tab shows exactly the one status it names", + arguments: [ + (WineFilter.new, WineStatus.new), + (WineFilter.want, WineStatus.want), + (WineFilter.bought, WineStatus.bought), + (WineFilter.tasted, WineStatus.tasted), + (WineFilter.not, WineStatus.not), + ]) + func eachTabIsOneStatus(filter: WineFilter, status: WineStatus) { + for candidate in WineStatus.allCases { + #expect(filter.matches(bottle(status: candidate)) == (candidate == status), + "\(filter.rawValue) vs \(candidate.rawValue)") + } + } + + /// FINDING: Vinnota/Model/Enums.swift:35 — `WineFilter` has no `maybe` + /// case, so a bottle marked "Undecided" appears under All and under nothing + /// else. Every other status has a tab. The three-way keenness control can + /// therefore file a bottle somewhere the cellar cannot filter back to. + @Test("FINDING: an Undecided bottle has no tab of its own") + func undecidedBottlesHaveNoTab() { + let undecided = bottle(status: .maybe) + #expect(undecided.statusLabel == "Undecided") + + let tabsShowingIt = WineFilter.allCases.filter { $0.matches(undecided) } + #expect(tabsShowingIt == [.all], "documented current behaviour") + + #expect(WineFilter.allCases.count == 6) + #expect(WineStatus.allCases.count == 6) + #expect(Set(WineFilter.allCases.map(\.rawValue)).contains("maybe") == false) + } + + /// The predicate compares raw values, so the two enums are coupled by + /// string. This pins the coupling down: renaming a `WineStatus` raw value + /// without renaming the matching `WineFilter` one would silently empty a tab. + @Test func tabsAreMatchedToStatusesByRawValue() { + for filter in WineFilter.allCases where filter != .all { + let matching = WineStatus.allCases.filter { $0.rawValue == filter.rawValue } + #expect(matching.count == 1, + "the \(filter.rawValue) tab must name a real status") + #expect(filter.matches(bottle(status: matching[0]))) + } + } + + @Test("Tab copy") + func tabCopy() { + #expect(WineFilter.all.label == "All") + #expect(WineFilter.new.label == "Scanned") + #expect(WineFilter.want.label == "Wanted") + #expect(WineFilter.bought.label == "In the rack") + #expect(WineFilter.tasted.label == "Tasted") + #expect(WineFilter.not.label == "Passed") + } + + /// The tab label and the chip on the card are allowed to differ — "Wanted" + /// vs "Want to try" — but the two that claim to be the same word must be. + @Test func tabLabelsAgreeWithChipsWhereTheyClaimTo() { + #expect(WineFilter.new.label == WineStatus.new.label) + #expect(WineFilter.bought.label == WineStatus.bought.label) + #expect(WineFilter.tasted.label == WineStatus.tasted.label) + #expect(WineFilter.not.label == WineStatus.not.label) + #expect(WineFilter.want.label != WineStatus.want.label) + } + + /// SearchView composes the two predicates with `&&`; both halves must hold. + /// This drives the same composition over a small book. + @Test func searchAndFilterCompose() { + let book = [ + bottle(producer: "Giuseppe Rinaldi", region: "Barolo", status: .bought), + bottle(producer: "G.D. Vajra", region: "Barolo", status: .tasted), + bottle(producer: "Clos Rougeard", region: "Saumur", status: .bought), + ] + + func results(_ query: String, _ filter: WineFilter) -> [String] { + book.filter { $0.matches(query: query) && filter.matches($0) }.map(\.producer) + } + + #expect(results("", .all).count == 3) + #expect(results("barolo", .all).count == 2) + #expect(results("barolo", .bought) == ["Giuseppe Rinaldi"]) + #expect(results("barolo", .tasted) == ["G.D. Vajra"]) + #expect(results("saumur", .tasted).isEmpty) + #expect(results(" ", .bought).count == 2, "a blank query leaves the tab alone") + #expect(results("riesling", .all).isEmpty) + } +} From 8de487566c3ff7fae834057e7a9329f76084e9a6 Mon Sep 17 00:00:00 2001 From: Sujoy Das Date: Sat, 5 Sep 2026 21:29:28 +0300 Subject: [PATCH 3/8] docs: keep README for end users and move developer notes to CLAUDE.md --- CLAUDE.md | 128 ++++++++++++++++++++++++++++++++++++++++++++++ README.md | 150 +++++++++++++++++++++--------------------------------- 2 files changed, 186 insertions(+), 92 deletions(-) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..a576dc3 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,128 @@ +# Vinnota — developer notes + +Native iOS app built from the Claude Design project +`Vinnota - Cellar Book.dc.html`. + +SwiftUI · iOS 17+ · SwiftData · Vision · Speech · AVFoundation · Swift Testing + +```bash +open Vinnota.xcodeproj +``` + +```bash +xcodebuild -scheme Vinnota -destination 'platform=iOS Simulator,name=iPhone 17 Pro' build +``` + +```bash +xcodebuild test -scheme Vinnota -destination 'platform=iOS Simulator,name=iPhone 17 Pro' +``` + +## Repository conventions + +**`main` is protected.** Work on a feature branch and open a PR; direct pushes +are rejected. + +**Commit messages** are enforced by a committed hook. Enable it once per clone: + +```bash +git config core.hooksPath .githooks +``` + +It requires a Conventional Commits subject of at most 100 characters, on a +**single line with no body**, and **no `Co-Authored-By` trailer**. Accepted +types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert. + +**CI** (`.github/workflows/ci.yml`) builds Debug and Release, fails on any Swift +warning, runs the test suite, and asserts the debug sign-in stub is absent from +the Release binary. It is explicitly `contents: read` and publishes nothing. +Actions are pinned to commit SHAs; Dependabot opens one grouped PR a month. + +## Layout + +``` +Vinnota/ + Theme/ Palette, Typography + Model/ Wine, TastingNote, enums, AppState, Settings, Formatters + Services/ AuthController, CameraController, LabelScanner, SpeechTranscriber + Views/ one file per screen, Sheets/, Components/ + Resources/ bundled fonts +VinnotaTests/ Swift Testing suites +``` + +`Vinnota.xcodeproj` uses file-system-synchronized groups (`objectVersion 77`), +so adding a Swift file to `Vinnota/` or `VinnotaTests/` is enough — no project +edit needed. + +## The screens + +Seven states on one surface, mirroring the design's single `screen` variable +rather than a navigation stack — every screen paints its own chrome. + +| Screen | File | What it does | +|---|---|---| +| Login | `LoginView` | Sign in with Apple, with a local stub fallback | +| Cellar | `CellarView` | Two-column grid, three stats, six filter tabs | +| Search | `SearchView` | Live filter over producer, cuvée, region, grape, shop | +| Scan | `ScanView` | Live camera + Vision OCR, photo-library fallback | +| Review | `ReviewView` | Correct what OCR read, then file the bottle | +| Detail | `DetailView` | Hero, facts, provenance timeline, notes | +| Tasting | `TastingView` | Photograph the pour, note it, pick a verdict | + +Plus four overlays: note composer, currency picker, purchase, and delete +confirmation — with a toast for confirmations. + +## The model + +A bottle moves `new → want / maybe / not → bought → tasted`. Prices are dual: +the shelf price seen when scanned, and what was actually paid. Notes are split +`pre` (before opening) and `post` (in the glass), each either typed or +dictated. + +Only the **producer** gates a save; everything else is optional and can be +filled in later from the edit screen. The reasoning is in OPEN-QUESTIONS §3.6a +— a form filled in a shop aisle that blocks on missing data produces no data, +not better data. + +The form's commit path lives on `WineForm` (`makeWine`, `apply(to:)`, +`makeNotes`) rather than inside `ReviewView.save()`, so it is reachable from +tests. It was inlined once, and a mutation dropping `.trimmed` went undetected. + +## What is real, not simulated + +The design fakes its scanner (a 1.6s delay and canned text) and its +transcription. Both are real here: + +- **`LabelScanner`** — `VNRecognizeTextRequest` at `.accurate`, five languages, + language correction off since labels are proper nouns. Fields are assigned by + heuristic: the tallest text is the producer, a four-digit year in range is the + vintage, and grape/region are matched against built-in lists. `parse(_:)` is + pure and directly unit-tested. +- **`SpeechTranscriber`** — `SFSpeechRecognizer`, preferring an on-device model + where one exists and otherwise falling back to Apple's servers. The waveform + is driven by real RMS levels off the audio buffer, not an animation. +- **`CameraController`** — `AVCaptureSession` with continuous autofocus. The + simulator has no camera, so `isAvailable` is false there and the scan screen + offers `PhotosPicker` into the identical OCR path. + +## Design fidelity + +Colours, type sizes, tracking, spacing and radii are transcribed from the +`.dc.html` rather than approximated. The palette lives in `Theme/Palette.swift` +with the source values in comments. + +Instrument Sans ships as a **variable** font, and iOS will not interpolate a +variable axis — `UIFont(name:size:)` always returns the default instance. So +weights are produced by driving the `wght` axis through CoreText +(`Theme/Typography.swift`). Instrument Serif is bundled as static regular and +italic faces from Google Fonts (OFL). + +## Unverified for want of hardware + +- **The camera path.** No camera on the development host, so scanning is tested + through the photo-library fallback — same OCR code, different image source. + CI runners have no camera either, so this gap is not closed by CI. +- **Dictation.** The Simulator cannot open an audio input on this virtualised + host, so `SpeechTranscriber` refuses there rather than letting AudioToolbox + abort the process. + +`TESTING.md` lists what to exercise on a real device. diff --git a/README.md b/README.md index d886761..3e58790 100644 --- a/README.md +++ b/README.md @@ -1,106 +1,72 @@ -# Vinnota — Cellar Book +# Cellar Book -A native iOS app built from the Claude Design project -`Vinnota - Cellar Book.dc.html`. Scan a wine label on the shelf, keep the note, -record what the bottle did in the glass. +A wine notebook for your phone. Point it at a label in the shop, keep the +bottle, and remember what it was actually like when you opened it. -SwiftUI · iOS 17+ · SwiftData · Vision · Speech · AVFoundation +Made for the moment you are standing in an aisle holding something you have +never heard of, trying to decide. --- -## Status +## What it does -**Builds and runs.** Verified on Xcode 26.6 / iOS 26.5, iPhone 17 Pro -simulator, 2026-09-05. The full flow was exercised: sign in, scan a label -through Vision OCR, correct and file it, set keenness, record a purchase, -taste it with a verdict, search, and delete. +**Scan a label.** Photograph the bottle and the producer, year, grape and +region are read off it for you. Correct anything it got wrong — labels are +hard to read, and it will not always get them right. -Two things are **not** verified, both for want of hardware: -- **The camera path.** No camera on this host, so scanning was tested through - the photo-library fallback — same OCR code, different image source. -- **Dictation.** The Simulator cannot open an audio input on this virtualised - host, so `SpeechTranscriber` refuses there rather than letting AudioToolbox - abort the process. Needs a real device. +**Or just type it in.** No camera, no signal, no patience: enter the name and +you are done. Everything else can wait, and you can add a label photo later. -See [OPEN-QUESTIONS.md](OPEN-QUESTIONS.md) for both, plus the missing login -photograph and the decisions taken along the way. +**Say how keen you are.** Want to try, undecided, or pass. The passes matter as +much as the wants — that is how you stop buying the same disappointing bottle +twice. -```bash -open Vinnota.xcodeproj -``` +**Keep a note.** Type it, or dictate it if your hands are full. What the +shopkeeper said, who recommended it, why you picked it up. -Or from the command line: +**Record the bottle you bought.** What you paid, how many, and where — which is +rarely what the shelf said. -```bash -xcodebuild -scheme Vinnota -destination 'platform=iOS Simulator,name=iPhone 17 Pro' build -``` +**Taste it.** Photograph the glass, write what it was like, and give it a +verdict. Loved, fine, or no. + +**Find it again.** Search across producer, cuvée, region, grape and shop, or +filter the shelf by where each bottle has got to. + +## Your cellar stays on your phone + +The book is stored on your device. There is no account to create beyond signing +in, nothing is uploaded, and no one else can see what you drink. + +Two things to know: + +- **It is not backed up anywhere by us.** Your cellar rides along in your normal + encrypted iPhone backup. Without one, losing the phone loses the book. +- **Dictation uses Apple's speech recognition.** On many phones that happens on + the device; where it cannot, Apple transcribes it. Either way the finished + note is kept on your phone and nowhere else. + +## Getting it running + +Requires an iPhone or iPad on **iOS 17 or later**. + +Open `Vinnota.xcodeproj` in Xcode, pick your device, and press run. Building to +a real iPhone needs a free Apple ID — [TESTING.md](TESTING.md) walks through the +signing set-up and what to try once it is installed. + +## Known gaps + +Being straight about what is not there yet: + +- **No export.** The only copy is on the phone. +- **The camera and dictation are unverified on real hardware.** They are written + and wired up, but the development machine has neither, so they have only been + exercised through the photo library and a simulator. +- **One cellar per device.** Signing out leaves the bottles behind, so a second + person on the same phone sees the first person's book. +- **English only, and no accessibility work yet** — text does not respond to + Larger Text, and there are no VoiceOver labels. --- -## The screens - -Seven states on one surface, mirroring the design's single `screen` variable -rather than a navigation stack — every screen paints its own chrome. - -| Screen | File | What it does | -|---|---|---| -| Login | `LoginView` | Sign in with Apple, with a local stub fallback | -| Cellar | `CellarView` | Two-column grid, three stats, six filter tabs | -| Search | `SearchView` | Live filter over producer, cuvée, region, grape, shop | -| Scan | `ScanView` | Live camera + Vision OCR, photo-library fallback | -| Review | `ReviewView` | Correct what OCR read, then file the bottle | -| Detail | `DetailView` | Hero, facts, provenance timeline, notes | -| Tasting | `TastingView` | Photograph the pour, note it, pick a verdict | - -Plus four overlays: dictation, currency picker, purchase, and delete -confirmation — with a toast for confirmations. - -## The model - -A bottle moves `new → want / maybe / not → bought → tasted`. Prices are dual: -the shelf price seen when scanned, and what was actually paid. Notes are split -`pre` (before opening) and `post` (in the glass), each either typed or -dictated. Tasted bottles cannot be deleted — they stay on the record. - -## What is real, not simulated - -The design fakes its scanner (a 1.6s delay and canned text) and its -transcription. Both are real here: - -- **`LabelScanner`** — `VNRecognizeTextRequest` at `.accurate`, five languages, - language correction off since labels are proper nouns. Fields are assigned by - heuristic: the tallest text is the producer, a four-digit year in range is the - vintage, and grape/region are matched against built-in lists. Boilerplate - ("contains sulfites", "75cl", appellation legalese) is filtered out. -- **`SpeechTranscriber`** — `SFSpeechRecognizer` with - `requiresOnDeviceRecognition`, honouring the design's on-device promise. The - waveform is driven by real RMS levels off the audio buffer, not an animation. -- **`CameraController`** — `AVCaptureSession` with continuous autofocus. The - simulator has no camera, so `isAvailable` is false there and the scan screen - offers `PhotosPicker` into the identical OCR path. - -## Design fidelity - -Colours, type sizes, tracking, spacing and radii are transcribed from the -`.dc.html` rather than approximated. The palette lives in `Theme/Palette.swift` -with the source values in comments. - -Instrument Sans ships as a **variable** font, and iOS will not interpolate a -variable axis — `UIFont(name:size:)` always returns the default instance. So -weights are produced by driving the `wght` axis through CoreText -(`Theme/Typography.swift`). Instrument Serif is bundled as static regular and -italic faces from Google Fonts (OFL). - -## Layout - -``` -Vinnota/ - Theme/ Palette, Typography - Model/ Wine, TastingNote, enums, AppState, Settings, Formatters - Services/ AuthController, CameraController, LabelScanner, SpeechTranscriber - Views/ one file per screen, Sheets/, Components/ - Resources/ bundled fonts -``` - -`Vinnota.xcodeproj` uses a file-system-synchronized group (`objectVersion 77`), -so adding a Swift file to `Vinnota/` is enough — no project edit needed. +Contributing or looking at the code? See [CLAUDE.md](CLAUDE.md). From 547c8d1caedd2f47b7f7984c16291524f21b7159 Mon Sep 17 00:00:00 2001 From: Sujoy Das Date: Sat, 5 Sep 2026 22:42:28 +0300 Subject: [PATCH 4/8] test: cover the wine model, search, persistence and app state transitions --- VinnotaTests/PersistenceAndStateTests.swift | 1532 +++++++++++++++++++ 1 file changed, 1532 insertions(+) create mode 100644 VinnotaTests/PersistenceAndStateTests.swift diff --git a/VinnotaTests/PersistenceAndStateTests.swift b/VinnotaTests/PersistenceAndStateTests.swift new file mode 100644 index 0000000..a510f95 --- /dev/null +++ b/VinnotaTests/PersistenceAndStateTests.swift @@ -0,0 +1,1532 @@ +import Foundation +import SwiftData +import Testing + +@testable import Vinnota + +// MARK: - Fixtures + +/// A store that behaves like the real one — same schema, same relationship and +/// cascade rules — but lives only for the test. +private func newContainer() throws -> ModelContainer { + try ModelContainer( + for: Schema([Wine.self, TastingNote.self]), + configurations: [ModelConfiguration(isStoredInMemoryOnly: true)] + ) +} + +/// A second `ModelContext` over the same store. Fetching through this one is a +/// genuine re-read: a single context hands back the instance it already has in +/// memory, which would let a field that never reached the store pass a +/// "round-trip" assertion. +private func reader(_ container: ModelContainer) -> ModelContext { + ModelContext(container) +} + +private func allWines(_ context: ModelContext) throws -> [Wine] { + try context.fetch(FetchDescriptor()) +} + +/// Notes are fetched as their own rows, never through `wine.notes` — an orphan +/// left behind by a failed cascade is invisible from the wine side by +/// definition. +private func allNotes(_ context: ModelContext) throws -> [TastingNote] { + try context.fetch(FetchDescriptor()) +} + +/// One megabyte with structure in it. A blob of repeated zeroes would survive a +/// truncation or a byte-order mangling unnoticed. +private func megabyte() -> Data { + let block = Data((0..<1024).map { UInt8($0 & 0xFF) }) + var out = Data(capacity: 1024 * 1024) + for _ in 0..<1024 { out.append(block) } + return out +} + +/// Every stored field set to something distinguishable, so a field dropped on +/// the way to the store shows up as a mismatch rather than as a coincidence. +private func fullyPopulated(labelPhoto: Data? = nil, pourPhoto: Data? = nil) -> Wine { + let wine = Wine(producer: "Giuseppe Rinaldi", name: "Brunate", vintage: "2019", + region: "Barolo", grape: "Nebbiolo", shop: "Enoteca Sciolla", + price: "68.50", currency: .CHF, labelPhoto: labelPhoto) + wine.addedByHand = true + wine.status = .tasted + wine.verdict = .loved + wine.boughtPrice = "62" + wine.boughtCurrency = .SEK + wine.boughtDate = "14 Aug 2026" + wine.qty = 3 + wine.scannedAt = "02 Aug 2026" + wine.openedAt = "14 Aug 2026" + wine.pourPhoto = pourPhoto + return wine +} + +// MARK: - Round-trip integrity + +@Suite("SwiftData round trip · every field") +struct RoundTripTests { + + @Test("A fully populated bottle comes back out of the store identical") + func everyFieldSurvives() throws { + let container = try newContainer() + let write = ModelContext(container) + + let original = fullyPopulated(labelPhoto: Data([0xFF, 0xD8, 0xFF, 0xE0]), + pourPhoto: Data([0x89, 0x50, 0x4E, 0x47])) + write.insert(original) + try write.save() + + let wines = try allWines(reader(container)) + #expect(wines.count == 1) + let back = try #require(wines.first) + + #expect(back.producer == "Giuseppe Rinaldi") + #expect(back.name == "Brunate") + #expect(back.vintage == "2019") + #expect(back.region == "Barolo") + #expect(back.grape == "Nebbiolo") + #expect(back.shop == "Enoteca Sciolla") + #expect(back.addedByHand == true) + #expect(back.statusRaw == WineStatus.tasted.rawValue) + #expect(back.status == .tasted) + #expect(back.verdictRaw == Verdict.loved.rawValue) + #expect(back.verdict == .loved) + #expect(back.price == "68.50") + #expect(back.currencyRaw == "CHF") + #expect(back.currency == .CHF) + #expect(back.boughtPrice == "62") + #expect(back.boughtCurrencyRaw == "SEK") + #expect(back.boughtCurrency == .SEK) + #expect(back.boughtDate == "14 Aug 2026") + #expect(back.qty == 3) + #expect(back.scannedAt == "02 Aug 2026") + #expect(back.openedAt == "14 Aug 2026") + #expect(back.labelPhoto == Data([0xFF, 0xD8, 0xFF, 0xE0])) + #expect(back.pourPhoto == Data([0x89, 0x50, 0x4E, 0x47])) + #expect(back.createdAt == original.createdAt) + } + + /// The distinction the whole price line rests on: `nil` means "no price and + /// none was ever entered", `""` means "the field was visited and left + /// blank". `displayPrice` renders them identically but `hasPrice` does not, + /// and a `nil` that came back as `""` would flip a fact on the detail page. + @Test("An absent price stays nil and does not come back as an empty string") + func absentOptionalsStayNil() throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = Wine(producer: "Rinaldi", name: "", vintage: "", region: "", + grape: "", shop: "") + write.insert(wine) + try write.save() + + let back = try #require(try allWines(reader(container)).first) + #expect(back.price == nil) + #expect(back.price != "") + #expect(back.boughtPrice == nil) + #expect(back.boughtDate == nil) + #expect(back.openedAt == nil) + #expect(back.verdictRaw == nil) + #expect(back.verdict == nil) + #expect(back.labelPhoto == nil) + #expect(back.pourPhoto == nil) + #expect(back.hasPrice == false) + } + + /// `currency` is a computed bridge over `currencyRaw`. Setting the enum must + /// land in the string column, and the string column is what the next launch + /// reads back. + @Test("Currency survives as its raw value in both currency columns", + arguments: CurrencyCode.allCases) + func currencyBridging(code: CurrencyCode) throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = Wine(producer: "Rinaldi", name: "", vintage: "", region: "", + grape: "", shop: "", currency: code) + wine.boughtCurrency = code + write.insert(wine) + try write.save() + + let back = try #require(try allWines(reader(container)).first) + #expect(back.currencyRaw == code.rawValue) + #expect(back.currency == code) + #expect(back.boughtCurrencyRaw == code.rawValue) + #expect(back.boughtCurrency == code) + } + + @Test("Every status round-trips through its raw column", + arguments: WineStatus.allCases) + func statusBridging(status: WineStatus) throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated() + wine.status = status + write.insert(wine) + try write.save() + + let back = try #require(try allWines(reader(container)).first) + #expect(back.statusRaw == status.rawValue) + #expect(back.status == status) + } + + @Test("Every verdict round-trips, and clearing it writes a real nil", + arguments: Verdict.allCases) + func verdictBridging(verdict: Verdict) throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated() + wine.verdict = verdict + write.insert(wine) + try write.save() + + let readBack = reader(container) + let back = try #require(try allWines(readBack).first) + #expect(back.verdictRaw == verdict.rawValue) + #expect(back.verdict == verdict) + + back.verdict = nil + try readBack.save() + #expect(try #require(try allWines(reader(container)).first).verdictRaw == nil) + } + + /// `createdAt` orders the note timeline, so sub-second precision is not + /// decoration — two notes taken in the same minute must not collapse. + @Test("Dates keep enough precision to order two notes taken together") + func datePrecision() throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated() + write.insert(wine) + let first = TastingNote(kind: .text, phase: .pre, text: "first") + let second = TastingNote(kind: .text, phase: .pre, text: "second") + second.createdAt = first.createdAt.addingTimeInterval(0.001) + first.wine = wine + second.wine = wine + write.insert(first) + write.insert(second) + try write.save() + + let notes = try allNotes(reader(container)).sorted { $0.createdAt < $1.createdAt } + #expect(notes.count == 2) + #expect(notes.first?.text == "first") + #expect(notes.last?.text == "second") + #expect(notes[0].createdAt < notes[1].createdAt) + } +} + +// MARK: - Photo blobs + +@Suite("Photo blobs · external storage") +struct PhotoBlobTests { + + /// `.externalStorage` moves large values out of the row and leaves a + /// reference behind. A blob that comes back short, reordered or nil is a + /// silent loss of the only copy of the label shot. + @Test("A megabyte label photo comes back byte for byte") + func largeBlobSurvives() throws { + let container = try newContainer() + let write = ModelContext(container) + + let blob = megabyte() + #expect(blob.count == 1_048_576) + + let wine = Wine(producer: "Rinaldi", name: "", vintage: "", region: "", + grape: "", shop: "", labelPhoto: blob) + write.insert(wine) + try write.save() + + let back = try #require(try allWines(reader(container)).first) + let out = try #require(back.labelPhoto) + #expect(out.count == blob.count) + #expect(out == blob) + #expect(out.first == blob.first) + #expect(out.last == blob.last) + } + + @Test("Both photo columns can hold a megabyte at once and stay distinct") + func twoLargeBlobsDoNotCrossOver() throws { + let container = try newContainer() + let write = ModelContext(container) + + var label = megabyte() + label[0] = 0xAA + var pour = megabyte() + pour[0] = 0xBB + + let wine = fullyPopulated(labelPhoto: label, pourPhoto: pour) + write.insert(wine) + try write.save() + + let back = try #require(try allWines(reader(container)).first) + #expect(back.labelPhoto?.first == 0xAA) + #expect(back.pourPhoto?.first == 0xBB) + #expect(back.labelPhoto == label) + #expect(back.pourPhoto == pour) + #expect(back.labelPhoto != back.pourPhoto) + } + + /// An empty `Data` is not the same fact as no data: the detail hero draws a + /// photo box for one and collapses for the other. `UIImage(data:)` fails on + /// both, so an empty blob shows an empty frame — but the model must at + /// least report it faithfully rather than folding it into nil. + @Test("An empty Data blob stays empty rather than becoming nil") + func emptyBlobIsNotNil() throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = Wine(producer: "Rinaldi", name: "", vintage: "", region: "", + grape: "", shop: "", labelPhoto: Data()) + write.insert(wine) + try write.save() + + let back = try #require(try allWines(reader(container)).first) + #expect(back.labelPhoto != nil, "documented current behaviour") + #expect(back.labelPhoto?.isEmpty == true) + #expect(back.labelPhoto?.count == 0) + } + + @Test("A photo can be replaced and cleared without disturbing the other one") + func blobsCanBeClearedIndependently() throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated(labelPhoto: Data([1, 2, 3]), pourPhoto: Data([9, 9])) + write.insert(wine) + try write.save() + + let editContext = reader(container) + let mid = try #require(try allWines(editContext).first) + mid.labelPhoto = Data([4, 5, 6, 7]) + try editContext.save() + #expect(try #require(try allWines(reader(container)).first).labelPhoto == Data([4, 5, 6, 7])) + + mid.labelPhoto = nil + try editContext.save() + let final = try #require(try allWines(reader(container)).first) + #expect(final.labelPhoto == nil) + #expect(final.pourPhoto == Data([9, 9]), "clearing one photo must not touch the other") + } + + /// Bytes that are not valid text, and bytes that would be mangled by any + /// string round trip. Photo data is arbitrary binary and must be treated as + /// such. + @Test("Non-UTF8 and null bytes survive a save") + func binarySafeBytes() throws { + let container = try newContainer() + let write = ModelContext(container) + + let blob = Data([0x00, 0xFF, 0xFE, 0x00, 0x80, 0xC0, 0x0A, 0x0D, 0x1A, 0x00]) + let wine = Wine(producer: "Rinaldi", name: "", vintage: "", region: "", + grape: "", shop: "", labelPhoto: blob) + write.insert(wine) + try write.save() + + #expect(try #require(try allWines(reader(container)).first).labelPhoto == blob) + } +} + +// MARK: - Relationships and cascade + +@Suite("Notes · relationship, split and cascade") +struct RelationshipTests { + + private func wineWithNotes(_ context: ModelContext) -> Wine { + let wine = fullyPopulated() + context.insert(wine) + for (index, spec) in [(TastingNote.Phase.pre, "smells like the shop"), + (.pre, "second thought before opening"), + (.post, "better in the second glass"), + (.post, "still going an hour later")].enumerated() { + let note = TastingNote(kind: index.isMultiple(of: 2) ? .text : .voice, + phase: spec.0, text: spec.1) + note.createdAt = Date().addingTimeInterval(Double(index)) + note.wine = wine + context.insert(note) + } + return wine + } + + @Test("Notes attached before the save are all there afterwards") + func notesSurviveTheSave() throws { + let container = try newContainer() + let write = ModelContext(container) + _ = wineWithNotes(write) + try write.save() + + let read = reader(container) + #expect(try allNotes(read).count == 4) + let back = try #require(try allWines(read).first) + #expect(back.notes.count == 4) + #expect(Set(back.notes.map(\.text)).count == 4) + } + + @Test("The inverse is populated in both directions after a re-read") + func inverseIsWiredBothWays() throws { + let container = try newContainer() + let write = ModelContext(container) + _ = wineWithNotes(write) + try write.save() + + let read = reader(container) + let wine = try #require(try allWines(read).first) + for note in try allNotes(read) { + #expect(note.wine != nil) + #expect(note.wine?.persistentModelID == wine.persistentModelID) + } + } + + @Test("preNotes and postNotes split by phase and sort by creation time") + func phaseSplitSurvivesTheStore() throws { + let container = try newContainer() + let write = ModelContext(container) + _ = wineWithNotes(write) + try write.save() + + let back = try #require(try allWines(reader(container)).first) + #expect(back.preNotes.count == 2) + #expect(back.postNotes.count == 2) + #expect(back.preNotes.map(\.text) == ["smells like the shop", + "second thought before opening"]) + #expect(back.postNotes.map(\.text) == ["better in the second glass", + "still going an hour later"]) + #expect(back.preNotes.allSatisfy { $0.phase == .pre }) + #expect(back.postNotes.allSatisfy { $0.phase == .post }) + #expect(back.preNotes.count + back.postNotes.count == back.notes.count) + } + + @Test("Note kind round-trips, so a typed note is never labelled Dictated") + func kindSurvivesTheStore() throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated() + write.insert(wine) + let typed = TastingNote(kind: .text, phase: .pre, text: "typed", when: "05 Sep · 14:32") + let spoken = TastingNote(kind: .voice, phase: .pre, text: "spoken", when: "05 Sep · 14:33") + typed.wine = wine + spoken.wine = wine + write.insert(typed) + write.insert(spoken) + try write.save() + + let notes = try allNotes(reader(container)) + let byText = Dictionary(uniqueKeysWithValues: notes.map { ($0.text, $0) }) + #expect(byText["typed"]?.kind == .text) + #expect(byText["typed"]?.label == "Typed · 05 Sep · 14:32") + #expect(byText["spoken"]?.kind == .voice) + #expect(byText["spoken"]?.label == "Dictated · 05 Sep · 14:33") + } + + /// The cascade is the reason deleting a bottle is safe at all. Asserted by + /// fetching `TastingNote` directly: an orphan is by construction not + /// reachable from the wine that no longer exists. + @Test("Deleting a wine cascades to its notes and leaves no orphans") + func cascadeLeavesNoOrphans() throws { + let container = try newContainer() + let write = ModelContext(container) + let wine = wineWithNotes(write) + try write.save() + #expect(try allNotes(reader(container)).count == 4) + + write.delete(wine) + try write.save() + + let after = reader(container) + #expect(try allWines(after).isEmpty) + #expect(try allNotes(after).isEmpty, "a surviving note is an orphan row") + } + + @Test("Deleting one bottle leaves the other bottle's notes untouched") + func cascadeIsScopedToOneWine() throws { + let container = try newContainer() + let write = ModelContext(container) + let doomed = wineWithNotes(write) + let keeper = wineWithNotes(write) + keeper.producer = "Keeper" + try write.save() + #expect(try allNotes(reader(container)).count == 8) + + write.delete(doomed) + try write.save() + + let after = reader(container) + #expect(try allWines(after).count == 1) + #expect(try allWines(after).first?.producer == "Keeper") + #expect(try allNotes(after).count == 4) + #expect(try allNotes(after).allSatisfy { $0.wine != nil }) + } + + @Test("Deleting a note does not take the bottle with it") + func deletingANoteDoesNotCascadeUpwards() throws { + let container = try newContainer() + let write = ModelContext(container) + _ = wineWithNotes(write) + try write.save() + + let read = reader(container) + let note = try #require(try allNotes(read).first) + read.delete(note) + try read.save() + + let after = reader(container) + #expect(try allWines(after).count == 1) + #expect(try allNotes(after).count == 3) + #expect(try #require(try allWines(after).first).notes.count == 3) + } + + @Test("Reassigning a note moves it between bottles rather than duplicating it") + func reassigningANoteDoesNotDuplicateIt() throws { + let container = try newContainer() + let write = ModelContext(container) + + let a = fullyPopulated() + a.producer = "A" + let b = fullyPopulated() + b.producer = "B" + write.insert(a) + write.insert(b) + let note = TastingNote(kind: .text, phase: .pre, text: "moves") + note.wine = a + write.insert(note) + try write.save() + + note.wine = b + try write.save() + + let after = reader(container) + #expect(try allNotes(after).count == 1) + let wines = try allWines(after) + #expect(wines.first { $0.producer == "A" }?.notes.isEmpty == true) + #expect(wines.first { $0.producer == "B" }?.notes.count == 1) + } +} + +// MARK: - Forward compatibility + +@Suite("Unknown raw values · an older build reading a newer store") +struct RawValueFallbackTests { + + /// A status written by a future build ("cellared", say) must not crash the + /// read. It falls back to `.new`. + @Test("An unknown statusRaw falls back to .new instead of trapping", + arguments: ["cellared", "", " ", "NEW", "new ", "1", "null", + "'; DROP TABLE ZWINE; --", "\u{1F377}"]) + func unknownStatus(raw: String) throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated() + write.insert(wine) + wine.statusRaw = raw + try write.save() + + let back = try #require(try allWines(reader(container)).first) + #expect(back.statusRaw == raw, "the unknown value is preserved, not rewritten") + #expect(back.status == .new) + #expect(back.verdict == .loved, "the verdict column is untouched and still supplies the accent") + } + + /// FINDING: Vinnota/Model/Wine.swift:80 — the fallback is `.new`, which is + /// the *most* permissive state. `canDelete` is `!isTasted` + /// (Wine.swift:155) and `isTasted` reads the fallback, so a bottle whose + /// status column this build cannot parse gets the trash icon back even if + /// it was drunk. The guard "tasted bottles stay on the record" is only as + /// strong as the raw string. + /// + /// Reachability: this build never writes an unparseable status — the only + /// writers are `Wine.init` and the `status` setter, both of which write a + /// `WineStatus.rawValue`. It takes a store written by a later build with a + /// new case (then opened by this one), or a damaged row. That is a real + /// forward-compatibility path for a local SwiftData store that survives app + /// updates, but it is not reachable from this build alone. The safe + /// fallback for a *guard* is the restrictive end: an unreadable status + /// should not re-arm deletion. + @Test("FINDING: an unknown status re-arms the delete button on a tasted bottle") + func unknownStatusReopensDeletion() throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated() + #expect(wine.status == .tasted) + #expect(wine.canDelete == false) + + write.insert(wine) + wine.statusRaw = "cellared" + try write.save() + + let back = try #require(try allWines(reader(container)).first) + #expect(back.status == .new, "documented current behaviour") + #expect(back.isTasted == false) + #expect(back.canDelete, "documented current behaviour — deletion is re-armed") + // `fullyPopulated` sets `addedByHand`, so `.new` renders as the + // hand-entry chip here; a scanned bottle would read "Scanned". Either + // way the chip claims the bottle was never opened. + #expect(back.statusLabel == "Added by hand") + } + + /// `WineFilter.matches` compares against `wine.status.rawValue` — the + /// fallback — and not against the stored column, so a bottle carrying a + /// status this build cannot read is filed under "Scanned" rather than + /// disappearing from every tab. Losing a bottle from the cellar list would + /// be worse; being told a tasted bottle was merely scanned is what actually + /// happens. + @Test("An unknown statusRaw files the bottle under the Scanned tab") + func unknownStatusFiltersAsScanned() throws { + let wine = fullyPopulated() + wine.statusRaw = "cellared" + #expect(WineFilter.all.matches(wine), "never lost from the cellar list") + #expect(WineFilter.new.matches(wine), "documented current behaviour") + for filter in WineFilter.allCases where filter != .all && filter != .new { + #expect(filter.matches(wine) == false, + "\(filter.rawValue) must not claim a bottle it cannot read") + } + #expect(WineFilter.tasted.matches(wine) == false, + "though the bottle was tasted before this build read it") + } + + @Test("An unknown verdictRaw reads as no verdict rather than trapping", + arguments: ["adored", "", "LOVED", "loved ", "0", "\u{202E}dellac"]) + func unknownVerdict(raw: String) throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated() + write.insert(wine) + wine.verdictRaw = raw + try write.save() + + let back = try #require(try allWines(reader(container)).first) + #expect(back.verdictRaw == raw) + #expect(back.verdict == nil) + #expect(back.status == .tasted, "the status column is untouched and now supplies the accent") + } + + /// A currency this build does not know must not silently reprice the + /// bottle. It cannot: the amount is a string and only the symbol changes. + @Test("An unknown currencyRaw falls back to EUR and leaves the amount alone", + arguments: ["JPY", "XBT", "", "eur", "EUR ", "$", "€"]) + func unknownCurrency(raw: String) throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated() + write.insert(wine) + wine.currencyRaw = raw + wine.boughtCurrencyRaw = raw + try write.save() + + let back = try #require(try allWines(reader(container)).first) + #expect(back.currencyRaw == raw) + #expect(back.currency == .EUR) + #expect(back.boughtCurrency == .EUR) + #expect(back.price == "68.50", "the amount is untouched by the currency fallback") + #expect(back.boughtPrice == "62") + #expect(back.displayPrice == "€62", "shown in the fallback currency, not the unknown one") + } + + /// FINDING: Vinnota/Model/Wine.swift:88 — an unknown currency is displayed + /// as euros with no marker that the code was not understood. A CHF bottle + /// whose currency column is corrupted prints "€62", which is a wrong number + /// rather than a missing one. + @Test("FINDING: an unknown currency silently reprints the amount as euros") + func unknownCurrencyMisstatesTheCurrency() throws { + let wine = fullyPopulated() + wine.boughtCurrencyRaw = "JPY" + #expect(wine.boughtPrice == "62") + #expect(wine.displayPrice == "€62", "documented current behaviour") + #expect(wine.displayPrice != "¥62") + #expect(wine.displayPrice.contains("JPY") == false) + } + + @Test("An unknown note kind or phase falls back rather than trapping", + arguments: ["shouted", "", "TEXT", "pre "]) + func unknownNoteRaws(raw: String) throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated() + write.insert(wine) + let note = TastingNote(kind: .voice, phase: .post, text: "n") + note.wine = wine + write.insert(note) + note.kindRaw = raw + note.phaseRaw = raw + try write.save() + + let back = try #require(try allNotes(reader(container)).first) + #expect(back.kind == .text, "kind falls back to .text") + #expect(back.phase == .pre, "phase falls back to .pre") + #expect(back.label.hasPrefix("Typed · ")) + } + + /// The phase fallback is not free: a post-tasting note whose phase column is + /// unreadable is re-filed under "Before opening" on the detail page. + @Test("An unreadable phase moves a post note into the pre column") + func unknownPhaseMovesTheNote() throws { + let wine = fullyPopulated() + let note = TastingNote(kind: .text, phase: .post, text: "in the glass") + note.wine = wine + wine.notes = [note] + #expect(wine.postNotes.count == 1) + + note.phaseRaw = "during" + #expect(wine.postNotes.isEmpty, "documented current behaviour") + #expect(wine.preNotes.count == 1) + } + + /// Writing through the typed property normalises a corrupt column, which is + /// the only self-healing the model has. + @Test("Assigning the typed property rewrites a corrupt raw column") + func writingThroughTheBridgeHeals() throws { + let wine = fullyPopulated() + wine.statusRaw = "cellared" + wine.currencyRaw = "JPY" + wine.verdictRaw = "adored" + + wine.status = .bought + wine.currency = .GBP + wine.verdict = .meh + + #expect(wine.statusRaw == "bought") + #expect(wine.currencyRaw == "GBP") + #expect(wine.verdictRaw == "meh") + } +} + +// MARK: - Store growth + +@Suite("Store growth · no duplicate rows") +struct StoreGrowthTests { + + @Test("Saving the same bottle ten times leaves one row") + func repeatedSavesDoNotDuplicate() throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated() + write.insert(wine) + for index in 0..<10 { + wine.qty = index + try write.save() + } + + let after = reader(container) + #expect(try allWines(after).count == 1) + #expect(try allWines(after).first?.qty == 9) + } + + @Test("Inserting the same instance repeatedly does not duplicate it") + func repeatedInsertsDoNotDuplicate() throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated() + for _ in 0..<5 { write.insert(wine) } + try write.save() + + #expect(try allWines(reader(container)).count == 1) + } + + /// A photo rewritten on every save — which is what re-picking a pour photo + /// does — must replace the blob, not accumulate copies visible as extra + /// rows. + @Test("Rewriting the photo repeatedly keeps one row and the last blob") + func rewritingBlobsDoesNotGrowTheStore() throws { + let container = try newContainer() + let write = ModelContext(container) + + let wine = fullyPopulated() + write.insert(wine) + for index in UInt8(0).. AppState { + let app = AppState() + app.screen = .tasting + app.sheet = .bought + app.selected = fullyPopulated() + app.filter = .bought + app.searchFilter = .tasted + app.query = "rinaldi" + app.currencyTarget = .buy + app.voiceTarget = .tasting + app.form.producer = "Half typed" + app.form.price = "45" + app.form.notes = [(when: "05 Sep · 14:32", text: "draft", typed: true)] + app.buy.price = "62" + app.buy.shop = "Enoteca" + app.tasting.text = "half a tasting note" + app.tasting.verdict = .loved + app.editing = fullyPopulated() + return app + } + + @Test("Navigation, selection and search all go back to their defaults") + func navigationIsCleared() { + let app = dirtied() + app.reset() + #expect(app.screen == .cellar) + #expect(app.sheet == nil) + #expect(app.selected == nil) + #expect(app.filter == .all) + #expect(app.searchFilter == .all) + #expect(app.query.isEmpty) + } + + @Test("Every draft form is emptied") + func draftsAreCleared() { + let app = dirtied() + app.reset() + #expect(app.form.producer.isEmpty) + #expect(app.form.price.isEmpty) + #expect(app.form.notes.isEmpty) + #expect(app.form.labelPhoto == nil) + #expect(app.buy.price.isEmpty) + #expect(app.buy.shop.isEmpty) + #expect(app.buy.qty == "1") + #expect(app.tasting.text.isEmpty) + #expect(app.tasting.verdict == nil) + #expect(app.tasting.pourPhoto == nil) + #expect(app.tasting.notes.isEmpty) + } + + /// FINDING: Vinnota/Model/AppState.swift:73 — `reset()` clears `form`, + /// `buy`, `tasting` and `selected`, but never `editing`. The comment on the + /// method says it exists "so a later session does not resume into the + /// previous user's half-finished form", and this is exactly the pointer + /// that breaks that promise. It also keeps the signed-out user's `Wine` + /// alive for the whole of the next session. + /// + /// NOT REACHABLE TODAY, and the difference matters. Signing out runs only + /// from `AccountSheet` (Vinnota/Views/Sheets/AccountSheet.swift:101), which + /// is presented from exactly one place — the cellar header avatar + /// (Vinnota/Views/CellarView.swift:33). `editing` is set in exactly one + /// place, the detail screen's pencil (Vinnota/Views/DetailView.swift:101), + /// which moves straight to `.review` in the same closure; and both ways off + /// the review screen — `leave()` (ReviewView.swift:108) and the editing + /// branch of `save()` (ReviewView.swift:262) — nil it out before + /// navigating. So no route reaches the cellar, and therefore the sign-out + /// button, with `editing` still set. + /// + /// It is a latent hazard rather than a live one: the moment a second entry + /// point to the account sheet appears (a search-screen avatar, a settings + /// row on the review screen, a revoked-credential sign-out), the write in + /// `staleEditingSurvivesAFreshForm` below becomes real. `reset()` should + /// clear `editing` regardless — every other draft pointer it holds is + /// cleared, and this is the only one whose survival can overwrite a row. + @Test("FINDING: reset() leaves `editing` pointing at the previous bottle") + func resetDoesNotClearEditing() { + let app = dirtied() + let stale = try? #require(app.editing) + app.reset() + #expect(app.editing != nil, "documented current behaviour") + #expect(app.editing === stale, "still the pre-sign-out bottle") + #expect(app.selected == nil, "while `selected` — the read-only pointer — was cleared") + } + + /// The same pointer viewed from the other side: a fresh form plus a stale + /// `editing` is precisely the state the scanner hands the Review screen + /// (Vinnota/Views/ScanView.swift:147 and :197 replace `form` without + /// touching `editing`). The overwrite is spelled out by calling + /// `apply(to:)` directly, which is the line `ReviewView.save()` runs when + /// `app.editing != nil` — the view itself is not exercised, and, as the + /// test above records, no real navigation reaches this state today. + @Test("FINDING: a reset state still reads as 'editing' to the Review screen") + func staleEditingSurvivesAFreshForm() { + let app = dirtied() + let victim = try? #require(app.editing) + app.reset() + + // What ScanView does on the way to the review screen. + app.form = WineForm() + app.form.producer = "A completely different producer" + app.go(.review) + + #expect(app.editing != nil, "documented current behaviour: isEditing is true") + #expect(app.editing === victim) + + // And what ReviewView.save() would then do with it. + app.form.apply(to: try! #require(app.editing)) + #expect(victim?.producer == "A completely different producer", + "the wrong bottle has been overwritten") + } + + /// `reset()` drops the app's reference to the bottle; it must not delete it. + @Test("reset() does not touch the store") + func resetDoesNotDeleteAnything() throws { + let container = try newContainer() + let context = ModelContext(container) + let wine = fullyPopulated() + context.insert(wine) + try context.save() + + let app = AppState() + app.selected = wine + app.editing = wine + app.reset() + + #expect(try allWines(reader(container)).count == 1) + } +} + +// MARK: - BuyForm + +@Suite("BuyForm") +struct BuyFormTests { + + /// The sheet opens pre-filled: today's date, one bottle, no price yet. + @Test("Defaults match what the sheet is expected to show") + func defaults() { + let form = BuyForm() + #expect(form.date == Formatters.today()) + #expect(form.qty == "1") + #expect(form.price.isEmpty) + #expect(form.currency == .EUR) + #expect(form.shop.isEmpty) + } +} + +// MARK: - TastingForm + +@Suite("TastingForm") +struct TastingFormTests { + + @Test("Defaults are empty across the board") + func defaults() { + let form = TastingForm() + #expect(form.text.isEmpty) + #expect(form.verdict == nil) + #expect(form.pourPhoto == nil) + #expect(form.notes.isEmpty) + } + + /// Every stored property, one by one. A field added later and left out of + /// `reset()` would leak the previous bottle's tasting into the next one — + /// the pour photo most visibly, since it becomes the detail hero. + @Test("reset() clears every field, enumerated") + func resetClearsEverything() { + var form = TastingForm() + form.text = "Tar and roses, and then some." + form.verdict = .disliked + form.pourPhoto = Data([1, 2, 3, 4]) + form.notes = [(when: "05 Sep · 20:10", text: "first glass", typed: false), + (when: "05 Sep · 21:40", text: "second glass", typed: true)] + + form.reset() + + #expect(form.text.isEmpty) + #expect(form.text == "") + #expect(form.verdict == nil) + #expect(form.pourPhoto == nil) + #expect(form.notes.isEmpty) + #expect(form.notes.count == 0) + } + + @Test("Resetting through AppState clears the draft on the state itself") + func resetThroughAppState() async { + await MainActor.run { + let app = AppState() + app.tasting.text = "leaked" + app.tasting.verdict = .meh + app.tasting.pourPhoto = Data([9]) + app.tasting.notes = [(when: "w", text: "t", typed: false)] + app.tasting.reset() + #expect(app.tasting.text.isEmpty) + #expect(app.tasting.verdict == nil) + #expect(app.tasting.pourPhoto == nil) + #expect(app.tasting.notes.isEmpty) + } + } +} + +// MARK: - WineForm defaults + +@Suite("WineForm defaults and photo carry-over") +struct WineFormDefaultsTests { + + @Test("A new form is empty and not marked as recognised") + func defaults() { + let form = WineForm() + #expect(form.producer.isEmpty) + #expect(form.name.isEmpty) + #expect(form.vintage.isEmpty) + #expect(form.region.isEmpty) + #expect(form.grape.isEmpty) + #expect(form.shop.isEmpty) + #expect(form.price.isEmpty) + #expect(form.currency == .EUR) + #expect(form.text.isEmpty) + #expect(form.recognized == false) + #expect(form.labelPhoto == nil) + #expect(form.notes.isEmpty) + } + + /// A photo-carrying form is not "by hand" even when OCR read nothing, so a + /// picked library photo is not described as a typed-in bottle. + @Test("addedByHand depends on both recognition and the photo", + arguments: [(false, false, true), (true, false, false), + (false, true, false), (true, true, false)]) + func handEntryFlag(recognized: Bool, hasPhoto: Bool, expected: Bool) { + var form = WineForm() + form.producer = "Rinaldi" + form.recognized = recognized + form.labelPhoto = hasPhoto ? Data([1]) : nil + #expect(form.makeWine().addedByHand == expected) + } + + /// An empty photo is not nil, so it counts as "carrying a label" and + /// suppresses the hand-entry flag even though nothing can be drawn from it. + @Test("FINDING: an empty photo blob suppresses the 'Added by hand' chip") + func emptyPhotoSuppressesHandEntry() { + var form = WineForm() + form.producer = "Rinaldi" + form.labelPhoto = Data() + let wine = form.makeWine() + #expect(wine.addedByHand == false, "documented current behaviour") + #expect(wine.statusLabel == "Scanned", "though nothing was scanned") + } + + @Test("The label photo is carried into the saved bottle intact") + func photoReachesTheBottle() throws { + let container = try newContainer() + let context = ModelContext(container) + + var form = WineForm() + form.producer = "Rinaldi" + form.labelPhoto = megabyte() + let wine = form.makeWine() + context.insert(wine) + try context.save() + + #expect(try #require(try allWines(reader(container)).first).labelPhoto?.count == 1_048_576) + } + + /// `apply(to:)` is the edit path. It must overwrite the photo column too, + /// otherwise an edited bottle keeps a photo the form no longer holds. + @Test("apply(to:) writes the photo column, including clearing it") + func applyWritesThePhoto() { + let wine = fullyPopulated(labelPhoto: Data([1, 2, 3])) + var form = WineForm(editing: wine) + form.labelPhoto = nil + form.apply(to: wine) + #expect(wine.labelPhoto == nil) + } +} + +// MARK: - Formatters + +@Suite("Formatters") +struct FormatterTests { + + /// Built through `Calendar.current` so the assertion holds in any timezone + /// the test machine happens to be in. + private func date(_ year: Int, _ month: Int, _ day: Int, + _ hour: Int = 0, _ minute: Int = 0) -> Date { + var parts = DateComponents() + parts.year = year + parts.month = month + parts.day = day + parts.hour = hour + parts.minute = minute + return Calendar.current.date(from: parts)! + } + + @Test("today() is two-digit day, three-letter month, four-digit year") + func todayShape() { + #expect(Formatters.today(date(2026, 9, 5)) == "05 Sep 2026") + #expect(Formatters.today(date(2026, 1, 1)) == "01 Jan 2026") + #expect(Formatters.today(date(2026, 12, 31)) == "31 Dec 2026") + } + + @Test("Every month maps to its own three-letter abbreviation") + func everyMonth() { + let expected = ["Jan", "Feb", "Mar", "Apr", "May", "Jun", + "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"] + for month in 1...12 { + #expect(Formatters.today(date(2026, month, 15)) == "15 \(expected[month - 1]) 2026") + } + } + + @Test("stamp() is day, month, a middle dot and a 24-hour clock") + func stampShape() { + #expect(Formatters.stamp(date(2026, 9, 5, 14, 32)) == "05 Sep · 14:32") + #expect(Formatters.stamp(date(2026, 8, 12, 18, 40)) == "12 Aug · 18:40") + } + + /// A note taken at midnight must read "00:00", not "12:00" and not "0:0". + @Test("stamp() zero-pads both clock fields and does not go to 12-hour") + func stampPadding() { + #expect(Formatters.stamp(date(2026, 3, 1, 0, 0)) == "01 Mar · 00:00") + #expect(Formatters.stamp(date(2026, 3, 1, 0, 5)) == "01 Mar · 00:05") + #expect(Formatters.stamp(date(2026, 3, 1, 9, 9)) == "01 Mar · 09:09") + #expect(Formatters.stamp(date(2026, 3, 1, 23, 59)) == "01 Mar · 23:59") + #expect(Formatters.stamp(date(2026, 3, 1, 13, 0)) == "01 Mar · 13:00") + } + + @Test("stamp() uses a middle dot, not a bullet or a hyphen") + func stampSeparator() { + let out = Formatters.stamp(date(2026, 9, 5, 14, 32)) + #expect(out.contains("\u{00B7}")) + #expect(out.contains("\u{2022}") == false) + #expect(out.contains(" - ") == false) + } + + @Test("A leap day formats without special-casing") + func leapDay() { + #expect(Formatters.today(date(2028, 2, 29)) == "29 Feb 2028") + #expect(Formatters.stamp(date(2028, 2, 29, 12, 0)) == "29 Feb · 12:00") + } + + @Test("Years outside the current century are printed as they are") + func unusualYears() { + #expect(Formatters.today(date(1999, 12, 31)) == "31 Dec 1999") + #expect(Formatters.today(date(2100, 1, 1)) == "01 Jan 2100") + } + + /// `stamp()` is the default for a note's `when`, and both are stored as + /// plain strings, so what they produce is what the store keeps forever. + @Test("A note stamps itself with the same string Formatters produces") + func noteUsesTheStamp() throws { + let container = try newContainer() + let context = ModelContext(container) + + let wine = fullyPopulated() + context.insert(wine) + let note = TastingNote(kind: .voice, phase: .post, text: "n") + note.wine = wine + context.insert(note) + try context.save() + + let back = try #require(try allNotes(reader(container)).first) + #expect(back.when == Formatters.stamp(back.createdAt)) + #expect(back.label == "Dictated · " + back.when) + } +} + +// MARK: - Settings + +/// These read and write `UserDefaults.standard`, so they run one at a time and +/// each restores what it found. The key strings are duplicated from +/// `Settings.Key`, which is private — they are the on-disk contract, so a +/// change to one of them is a migration and worth failing on. +@Suite("Settings · defaults and persistence", .serialized) +struct SettingsTests { + + private static let keys = ["defaultCurrency", "showShelfPrice", "labelRecognition"] + + private func withCleanDefaults(_ body: () throws -> Void) rethrows { + let defaults = UserDefaults.standard + let saved = Self.keys.map { defaults.object(forKey: $0) } + for key in Self.keys { defaults.removeObject(forKey: key) } + defer { + for (key, value) in zip(Self.keys, saved) { + if let value { defaults.set(value, forKey: key) } + else { defaults.removeObject(forKey: key) } + } + } + try body() + } + + @Test("With nothing stored, the defaults are EUR and both toggles on") + func unsetDefaults() { + withCleanDefaults { + #expect(Settings.defaultCurrency == .EUR) + #expect(Settings.showShelfPrice == true) + #expect(Settings.labelRecognition == true) + } + } + + @Test("Each currency survives a write and a read") + func currencyPersists() { + withCleanDefaults { + for code in CurrencyCode.allCases { + Settings.defaultCurrency = code + #expect(Settings.defaultCurrency == code) + #expect(UserDefaults.standard.string(forKey: "defaultCurrency") == code.rawValue) + } + } + } + + /// The stored value is a raw string, so a build that no longer knows a code + /// must fall back rather than trap — the same forward-compatibility seam as + /// the store columns. + @Test("An unreadable stored currency falls back to EUR") + func corruptCurrencyFallsBack() { + withCleanDefaults { + for junk in ["JPY", "", "eur", "42", "€"] { + UserDefaults.standard.set(junk, forKey: "defaultCurrency") + #expect(Settings.defaultCurrency == .EUR) + } + } + } + + /// A non-string value under the key must not crash the getter either. + @Test("A wrong-typed stored currency falls back to EUR") + func wrongTypedCurrencyFallsBack() { + withCleanDefaults { + UserDefaults.standard.set(42, forKey: "defaultCurrency") + #expect(Settings.defaultCurrency == .EUR) + UserDefaults.standard.set(["EUR"], forKey: "defaultCurrency") + #expect(Settings.defaultCurrency == .EUR) + } + } + + /// The toggles default to `true`, so they must distinguish "never set" from + /// "set to false" — an `object(forKey:) as? Bool` that fell through to + /// `bool(forKey:)` would turn both defaults off. + @Test("Both toggles distinguish 'never set' from 'set to false'") + func togglesRoundTrip() { + withCleanDefaults { + #expect(Settings.showShelfPrice == true) + Settings.showShelfPrice = false + #expect(Settings.showShelfPrice == false) + Settings.showShelfPrice = true + #expect(Settings.showShelfPrice == true) + + #expect(Settings.labelRecognition == true) + Settings.labelRecognition = false + #expect(Settings.labelRecognition == false) + Settings.labelRecognition = true + #expect(Settings.labelRecognition == true) + } + } + + @Test("The two toggles are independent keys") + func togglesAreIndependent() { + withCleanDefaults { + Settings.showShelfPrice = false + #expect(Settings.labelRecognition == true) + Settings.labelRecognition = false + Settings.showShelfPrice = true + #expect(Settings.showShelfPrice == true) + #expect(Settings.labelRecognition == false) + } + } + + /// The scanner seeds a new form's currency from this preference, which is + /// the only place the setting reaches the model. + @Test("The stored currency is what a scanned form starts in") + func settingSeedsTheForm() { + withCleanDefaults { + Settings.defaultCurrency = .SEK + let reading = LabelReading(producer: "Rinaldi", name: "", vintage: "2019", + region: "", grape: "", recognized: true) + let form = WineForm(reading: reading, photo: nil, + currency: Settings.defaultCurrency) + #expect(form.currency == .SEK) + #expect(form.makeWine().currency == .SEK) + #expect(form.makeWine().boughtCurrency == .SEK) + } + } +} From 96950a33012631245c7610874cabe8cb0bfe867c Mon Sep 17 00:00:00 2001 From: Sujoy Das Date: Sat, 5 Sep 2026 22:42:28 +0300 Subject: [PATCH 5/8] test: cover auth, session handling and the shipped privacy configuration --- VinnotaTests/AuthSecurityTests.swift | 1293 ++++++++++++++++++++++++++ 1 file changed, 1293 insertions(+) create mode 100644 VinnotaTests/AuthSecurityTests.swift diff --git a/VinnotaTests/AuthSecurityTests.swift b/VinnotaTests/AuthSecurityTests.swift new file mode 100644 index 0000000..0c51011 --- /dev/null +++ b/VinnotaTests/AuthSecurityTests.swift @@ -0,0 +1,1293 @@ +import AuthenticationServices +import Foundation +import ObjectiveC +import Security +import Testing +import UIKit + +@testable import Vinnota + +// MARK: - The storage contract, restated + +/// `AuthController` keeps every one of these private, which is right for the +/// app and useless for a test: an assertion against `auth.displayName` proves +/// only that a getter agrees with its own setter. These literals are the real +/// on-disk contract — the keychain account, the two `UserDefaults` keys, the +/// avatar's filename, and the user ID that marks a session as stubbed — so the +/// tests below read and write *storage* and let the controller observe it. +/// +/// A change to any of them is a migration: the old values keep living on the +/// device, and these tests are meant to fail when that happens. +private enum Storage { + static let keychainAccount = "com.vinnota.appleUserID" + static let stubUserID = "stub.local.account" + static let nameKey = "com.vinnota.displayName" + static let emailKey = "com.vinnota.email" + static let avatarFilename = "avatar.jpg" + + static var avatarURL: URL? { + try? FileManager.default.url(for: .applicationSupportDirectory, + in: .userDomainMask, + appropriateFor: nil, create: true) + .appendingPathComponent(avatarFilename) + } +} + +// MARK: - Keychain, driven directly + +/// The same generic-password item `AuthController` writes, reached from the +/// outside. Planting a value here is how "a previous launch left a session +/// behind" is simulated without going through Apple's UI. +private func keychainWrite(_ value: String) { + keychainDelete() + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrAccount as String: Storage.keychainAccount, + kSecValueData as String: Data(value.utf8), + kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlock, + ] + SecItemAdd(query as CFDictionary, nil) +} + +private func keychainRead() -> String? { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrAccount as String: Storage.keychainAccount, + kSecReturnData as String: true, + kSecMatchLimit as String: kSecMatchLimitOne, + ] + var item: CFTypeRef? + guard SecItemCopyMatching(query as CFDictionary, &item) == errSecSuccess, + let data = item as? Data else { return nil } + return String(data: data, encoding: .utf8) +} + +private func keychainDelete() { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrAccount as String: Storage.keychainAccount, + ] + SecItemDelete(query as CFDictionary) +} + +/// Whether this build can use the keychain at all. +/// +/// `xcodebuild … CODE_SIGNING_ALLOWED=NO` — what CI runs, and what the command +/// in TESTING.md runs — produces an unsigned application. An unsigned process +/// has no `application-identifier` entitlement, and the simulator keychain +/// answers every request from one with `errSecMissingEntitlement` (-34018). +/// `AuthController`'s own `SecItemAdd` fails there too, silently: it ignores +/// the status, so in this configuration the app signs in and then forgets the +/// session at the next launch. +/// +/// The tests that need a working keychain therefore say so and are skipped +/// rather than passing vacuously against a store that swallows everything. +/// Run the suite from Xcode, or with signing enabled, to exercise them. +private let keychainAvailable: Bool = { + // A private account name, so probing can never disturb the app's item. + let account = "com.vinnota.tests.keychainProbe" + let base: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrAccount as String: account, + ] + SecItemDelete(base as CFDictionary) + var add = base + add[kSecValueData as String] = Data("probe".utf8) + add[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock + let status = SecItemAdd(add as CFDictionary, nil) + SecItemDelete(base as CFDictionary) + return status == errSecSuccess +}() + +private let keychainSkipReason = Comment( + rawValue: "the keychain refuses unsigned builds with errSecMissingEntitlement; " + + "build with code signing enabled to exercise session persistence") + +/// Establishes a local session the way the app does. +/// +/// In DEBUG that is the stub itself. The stub does not exist in a Release +/// compile, so the credential is planted directly there — which keeps this +/// file compiling in both configurations, as the app target does. +@MainActor +private func establishLocalSession(_ auth: AuthController) { + #if DEBUG + auth.signInStubbed() + #else + keychainWrite(Storage.stubUserID) + #endif +} + +// MARK: - Global state, saved and put back + +/// Everything below writes the real `UserDefaults`, the real keychain and a +/// real file in Application Support — the same ones the running app uses. Each +/// test starts from a clean slate and hands back exactly what it found. +@MainActor +private func withCleanAuthState(_ body: () async throws -> Void) async throws { + let defaults = UserDefaults.standard + let savedName = defaults.object(forKey: Storage.nameKey) + let savedEmail = defaults.object(forKey: Storage.emailKey) + let savedSession = keychainRead() + let savedAvatar = Storage.avatarURL.flatMap { try? Data(contentsOf: $0) } + + defaults.removeObject(forKey: Storage.nameKey) + defaults.removeObject(forKey: Storage.emailKey) + keychainDelete() + if let url = Storage.avatarURL { try? FileManager.default.removeItem(at: url) } + + defer { + if let savedName { defaults.set(savedName, forKey: Storage.nameKey) } + else { defaults.removeObject(forKey: Storage.nameKey) } + if let savedEmail { defaults.set(savedEmail, forKey: Storage.emailKey) } + else { defaults.removeObject(forKey: Storage.emailKey) } + + if let savedSession { keychainWrite(savedSession) } else { keychainDelete() } + + if let url = Storage.avatarURL { + if let savedAvatar { try? savedAvatar.write(to: url, options: .atomic) } + else { try? FileManager.default.removeItem(at: url) } + } + } + + try await body() +} + +// MARK: - Image fixtures + +/// A JPEG at a known pixel size, drawn at scale 1 so its pixel dimensions and +/// its point dimensions are the same number. A renderer left on the device's +/// default scale would make every size assertion below three times ambiguous. +@MainActor +private func jpegFixture(_ width: Int, _ height: Int) -> Data { + let format = UIGraphicsImageRendererFormat.default() + format.scale = 1 + format.opaque = true + let size = CGSize(width: width, height: height) + let image = UIGraphicsImageRenderer(size: size, format: format).image { ctx in + UIColor.systemPink.setFill() + ctx.fill(CGRect(origin: .zero, size: size)) + // Structure, so a mangled or truncated file is not mistaken for a + // successful round trip. + UIColor.systemBlue.setFill() + ctx.fill(CGRect(x: 0, y: 0, width: width / 2, height: height / 2)) + } + return image.jpegData(compressionQuality: 1.0) ?? Data() +} + +/// The scale `AuthController.downscale` renders at — it builds a +/// `UIGraphicsImageRenderer` with no format, so it inherits the device's. +@MainActor +private var rendererScale: CGFloat { UIGraphicsImageRendererFormat.default().scale } + +// MARK: - The app bundle, as shipped + +/// The host application, not the test bundle. `AuthController` is compiled into +/// the app, so the bundle that contains it is the one that ships. +private var appBundle: Bundle { Bundle(for: AuthController.self) } + +// MARK: - Output capture + +/// Runs `body` with `stdout` and `stderr` pointed at a file, and returns what +/// was written. Used to assert a credential never reaches the console — the +/// app has no logger at all, and this is what keeps it that way. +@MainActor +private func capturingConsole(_ body: () async -> Void) async -> String { + let path = NSTemporaryDirectory() + "auth-console-\(UUID().uuidString).log" + FileManager.default.createFile(atPath: path, contents: nil) + guard let sink = FileHandle(forWritingAtPath: path) else { + await body() + return "" + } + let savedOut = dup(STDOUT_FILENO) + let savedErr = dup(STDERR_FILENO) + dup2(sink.fileDescriptor, STDOUT_FILENO) + dup2(sink.fileDescriptor, STDERR_FILENO) + + defer { + dup2(savedOut, STDOUT_FILENO) + dup2(savedErr, STDERR_FILENO) + close(savedOut) + close(savedErr) + try? sink.close() + try? FileManager.default.removeItem(atPath: path) + } + + await body() + + fflush(stdout) + fflush(stderr) + return (try? String(contentsOfFile: path, encoding: .utf8)) ?? "" +} + +// MARK: - The debug stub + +/// The app once shipped a sign-in stub that let anyone past authentication. +/// It now lives behind `#if DEBUG`, and CI greps the Release binary for its +/// symbol. Tests compile in DEBUG, so the stub is *present here* — which is +/// the only configuration in which its blast radius can actually be measured. +@Suite("Debug sign-in stub · blast radius", .serialized) +@MainActor +struct DebugStubTests { + + @Test("The keychain answers, so every keychain assertion below means something", + .enabled(if: keychainAvailable, keychainSkipReason)) + func keychainWorks() async throws { + try await withCleanAuthState { + // Without this, `keychainRead()` would return nil everywhere and + // half of this file would pass by accident. + let probe = "keychain-probe-\(UUID().uuidString)" + keychainWrite(probe) + #expect(keychainRead() == probe) + keychainDelete() + #expect(keychainRead() == nil) + } + } + + #if DEBUG + @Test("The stub signs in with no credential at all, and says so in the state") + func stubSignsInWithoutACredential() async throws { + try await withCleanAuthState { + let auth = AuthController() + #expect(auth.state == .signedOut) + + auth.signInStubbed() + + // The whole blast radius, stated exactly: a session exists, it is + // the one fixed local user ID, and it is flagged as not real. + #expect(auth.state == .signedIn(userID: Storage.stubUserID, stubbed: true)) + #expect(auth.isStubbedSession) + // It invents no identity — a stub account is nameless. + #expect(auth.displayName == nil) + #expect(auth.email == nil) + #expect(auth.avatar == nil) + } + } + + @Test("The stub writes its fixed user ID to the same keychain item a real session uses", + .enabled(if: keychainAvailable, keychainSkipReason)) + func stubPersistsToTheKeychain() async throws { + try await withCleanAuthState { + let auth = AuthController() + auth.signInStubbed() + + // This is the discriminator the release build keys off. It is a + // constant, not a random or per-device value, so a release build + // can recognise a stub left behind by a dev build with certainty. + #expect(keychainRead() == Storage.stubUserID) + } + } + + @Test("Signing out destroys the stub session rather than hiding it", + .enabled(if: keychainAvailable, keychainSkipReason)) + func signOutDestroysTheStub() async throws { + try await withCleanAuthState { + let auth = AuthController() + auth.signInStubbed() + #expect(keychainRead() == Storage.stubUserID) + + auth.signOut() + + #expect(auth.state == .signedOut) + #expect(auth.isStubbedSession == false) + #expect(keychainRead() == nil) + + // And a fresh launch does not find it again. + let relaunched = AuthController() + await relaunched.restore() + #expect(relaunched.state == .signedOut) + } + } + #endif + + @Test("A stubbed session and a real session with the same user ID are not equal") + func theStubbedFlagIsPartOfTheSessionIdentity() { + let stubbed = AuthController.State.signedIn(userID: Storage.stubUserID, stubbed: true) + let real = AuthController.State.signedIn(userID: Storage.stubUserID, stubbed: false) + // `AccountSheet` shows its "local account on this device only" warning + // off this flag. If equality ignored it, a stub could be assigned over + // a real session, or the reverse, without the UI noticing. + #expect(stubbed != real) + #expect(stubbed != .signedOut) + #expect(real != .signedOut) + } + + /// **The Release rejection branch is NOT covered by this suite.** + /// + /// `AuthController.restore()` handles a stub credential in two arms of an + /// `#if DEBUG` (AuthController.swift:111-120). Test bundles compile in + /// DEBUG, so only the DEBUG arm is ever built here — the `#else` arm that + /// destroys a stale stub in a shipped build is not merely unasserted, it is + /// not compiled. Confirmed by mutation: replacing that arm's + /// `deleteKeychain(); state = .signedOut` with + /// `state = .signedIn(userID: stored, stubbed: true)` — a shipped build + /// admitting a dev build's session with no credential at all — leaves the + /// whole 302-test run green. + /// + /// The `#else` block below is therefore a latent assertion, correct but + /// unreachable under `xcodebuild test`. What actually guards the release + /// build today is CI's `nm | grep signInStubbed` check, and + /// `theCIGuardStillHasSomethingToFind` below keeps that grep's needle + /// honest. Closing the gap properly means running this target against a + /// Release compile of the app; until then the name of this test claims + /// only what a DEBUG run really proves. + @Test("A stale stub credential is restored as a live session in DEBUG (Release arm not compiled here)", + .enabled(if: keychainAvailable, keychainSkipReason)) + func stubSessionAtLaunch() async throws { + try await withCleanAuthState { + // A stale stub left behind by a development build. + keychainWrite(Storage.stubUserID) + + let auth = AuthController() + await auth.restore() + + #if DEBUG + // Compiled here: the stub is a persistent session, not merely an + // in-memory convenience. That is its blast radius — it outlives + // the process that created it. + #expect(auth.state == .signedIn(userID: Storage.stubUserID, stubbed: true)) + #expect(auth.isStubbedSession) + #expect(keychainRead() == Storage.stubUserID) + #else + // The release path. Rejection alone is not enough: the stored + // credential has to be destroyed, or the next launch meets it + // again. Never reached by a standard test run — see above. + #expect(auth.state == .signedOut) + #expect(auth.isStubbedSession == false) + #expect(keychainRead() == nil) + #endif + } + } + + @Test("A near-miss of the stub user ID is not treated as a stub in any configuration", + .enabled(if: keychainAvailable, keychainSkipReason), + arguments: ["Stub.Local.Account", + "stub.local.account ", + " stub.local.account", + "stub.local.accounts", + "stub.local.accoun", + "xstub.local.account", + "stub·local·account"]) + func theStubIsMatchedExactly(_ planted: String) async throws { + try await withCleanAuthState { + keychainWrite(planted) + + let auth = AuthController() + await auth.restore() + + // The comparison is `==`, not a prefix or a case-insensitive + // match. Anything that is not exactly the stub goes down the real + // path, where an unverifiable ID is rejected — so the answer is + // the same in Debug and in Release. + #expect(auth.isStubbedSession == false) + #expect(auth.state == .signedOut) + #expect(keychainRead() == nil) + } + } + + @Test("An unverifiable Apple user ID is rejected and its credential destroyed", + .enabled(if: keychainAvailable, keychainSkipReason)) + func revokedCredentialIsNotTrusted() async throws { + try await withCleanAuthState { + // Shaped like a real Apple ID, belonging to nobody. + keychainWrite("001234.9f8e7d6c5b4a39281706abcdef012345.1122") + + let auth = AuthController() + await auth.restore() + + #expect(auth.state == .signedOut) + #expect(auth.isStubbedSession == false) + // `credentialState` is read through `try?`, so a thrown error and + // a genuine revocation land in the same branch. It fails closed, + // which is the safe direction — worth knowing that it also means + // a transient failure of the authorization daemon signs the user + // out for good rather than for one launch. + #expect(keychainRead() == nil) + } + } + + @Test("The symbol CI greps for in Release is present in this Debug build") + func theCIGuardStillHasSomethingToFind() throws { + // `.github/workflows/ci.yml` fails the Release build when + // `nm -a | grep signInStubbed` matches. That guard is only + // meaningful while the name it looks for is the name the stub has: + // rename the method and the grep passes forever, silently. + // + // The image is asked for by way of the runtime rather than taken as + // `Bundle.executableURL`, because Xcode 16 splits a Debug build into a + // 40 KB launcher and a `Vinnota.debug.dylib` holding all of the code. + // A Release build has no such split, which is why CI's simpler lookup + // is right for the binary it inspects. + let image = try #require(class_getImageName(AuthController.self)) + let path = String(cString: image) + let binary = try Data(contentsOf: URL(fileURLWithPath: path), options: .mappedIfSafe) + let present = binary.range(of: Data("signInStubbed".utf8)) != nil + + #if DEBUG + #expect(present, "CI's Release guard greps for a symbol that no longer exists") + #else + #expect(present == false, "the debug sign-in stub leaked into a Release build") + #endif + } + + @Test("FINDING (medium): rejecting a session at launch leaves the identity and photo on disk", + .enabled(if: keychainAvailable, keychainSkipReason)) + func rejectedSessionLeavesResidualIdentity() async throws { + try await withCleanAuthState { + // A previous session's leavings: name, email, profile photograph. + UserDefaults.standard.set("Marie Kondo", forKey: Storage.nameKey) + UserDefaults.standard.set("marie@example.com", forKey: Storage.emailKey) + let seeder = AuthController() + seeder.setAvatar(jpegFixture(200, 200)) + let avatarURL = try #require(Storage.avatarURL) + #expect(FileManager.default.fileExists(atPath: avatarURL.path)) + + // The credential is no longer good, so launch throws the session + // out. This is the same branch a Release build takes when it finds + // a stub session left behind by a dev build. + keychainWrite("001234.9f8e7d6c5b4a39281706abcdef012345.1122") + let auth = AuthController() + await auth.restore() + + #expect(auth.state == .signedOut) + #expect(keychainRead() == nil) + + // FINDING (medium): AuthController.swift:126-129 (the `default:` + // branch of `restore()`) and :114-118 (the Release stub branch) + // both delete the keychain item and return; neither calls + // `signOut()`. So the display name, the email address and the + // profile photograph outlive the session that produced them. + // In the shipped app: a device that ran a dev build and is then + // given a Release build shows the login screen while the dev + // account's email and photo are still on disk — and `restore()` + // has already loaded that photo into memory, because + // `loadAvatar()` runs at :108 before the session is checked at + // all. Asserted here as it is, not as it should be. + #expect(UserDefaults.standard.string(forKey: Storage.nameKey) == "Marie Kondo") + #expect(UserDefaults.standard.string(forKey: Storage.emailKey) == "marie@example.com") + #expect(FileManager.default.fileExists(atPath: avatarURL.path)) + #expect(auth.displayName == "Marie Kondo") + #expect(auth.email == "marie@example.com") + #expect(auth.avatar != nil, "restore() loads the avatar before it checks the session") + } + } +} + +// MARK: - Session persistence + +@Suite("Session persistence · what is stored and what sign-out clears", .serialized) +@MainActor +struct SessionPersistenceTests { + + @Test("displayName and email read the documented UserDefaults keys, not some other pair") + func identityIsReadFromTheDocumentedKeys() async throws { + try await withCleanAuthState { + let auth = AuthController() + #expect(auth.displayName == nil) + #expect(auth.email == nil) + + UserDefaults.standard.set("Marie Kondo", forKey: Storage.nameKey) + UserDefaults.standard.set("marie@example.com", forKey: Storage.emailKey) + + // Written to storage from outside, read back through the app. + #expect(auth.displayName == "Marie Kondo") + #expect(auth.email == "marie@example.com") + } + } + + @Test("The identity is not held in memory — a second controller sees the same values") + func identityIsSharedThroughStorage() async throws { + try await withCleanAuthState { + UserDefaults.standard.set("Jean Peridot", forKey: Storage.nameKey) + UserDefaults.standard.set("jean@example.com", forKey: Storage.emailKey) + + let first = AuthController() + let second = AuthController() + #expect(first.displayName == second.displayName) + #expect(first.email == second.email) + #expect(second.email == "jean@example.com") + } + } + + @Test("Sign-out clears the name, the email and the avatar from storage") + func signOutClearsTheIdentityAndThePicture() async throws { + try await withCleanAuthState { + let auth = AuthController() + establishLocalSession(auth) + UserDefaults.standard.set("Marie Kondo", forKey: Storage.nameKey) + UserDefaults.standard.set("marie@example.com", forKey: Storage.emailKey) + auth.setAvatar(jpegFixture(300, 300)) + + let avatarURL = try #require(Storage.avatarURL) + #expect(FileManager.default.fileExists(atPath: avatarURL.path)) + + auth.signOut() + + // In memory. + #expect(auth.state == .signedOut) + #expect(auth.isStubbedSession == false) + #expect(auth.displayName == nil) + #expect(auth.email == nil) + #expect(auth.avatar == nil) + + // And — the part that actually matters — in storage. A residual + // credential after sign-out is the defect being looked for here. + #expect(UserDefaults.standard.object(forKey: Storage.nameKey) == nil) + #expect(UserDefaults.standard.object(forKey: Storage.emailKey) == nil) + #expect(FileManager.default.fileExists(atPath: avatarURL.path) == false) + } + } + + @Test("Sign-out destroys the stored session as well", + .enabled(if: keychainAvailable, keychainSkipReason)) + func signOutClearsTheStoredSession() async throws { + try await withCleanAuthState { + let auth = AuthController() + establishLocalSession(auth) + #expect(keychainRead() != nil) + + auth.signOut() + #expect(keychainRead() == nil) + } + } + + @Test("Sign-out reaches the preferences store, not only the in-process cache") + func signOutReachesThePreferencesStore() async throws { + try await withCleanAuthState { + let key = Storage.emailKey as CFString + let address = "marie.kondo.private@example.com" + + let auth = AuthController() + establishLocalSession(auth) + UserDefaults.standard.set(address, forKey: Storage.emailKey) + CFPreferencesAppSynchronize(kCFPreferencesCurrentApplication) + + // `CFPreferencesCopyAppValue` goes to the preferences daemon, so + // it sees what was actually persisted rather than what + // `UserDefaults` is holding in this process. (The .plist on disk + // is not usable for this: cfprefsd writes it back on its own + // schedule, and it is still 42 bytes long moments after a write.) + // The precondition is an assertion too — if the address never + // reached the store, the check after sign-out would prove nothing. + let before = CFPreferencesCopyAppValue(key, kCFPreferencesCurrentApplication) as? String + #expect(before == address, "the email is persisted in the clear, unencrypted") + + auth.signOut() + CFPreferencesAppSynchronize(kCFPreferencesCurrentApplication) + + let after = CFPreferencesCopyAppValue(key, kCFPreferencesCurrentApplication) as? String + #expect(after == nil, "the address outlives sign-out in the preferences store") + } + } + + @Test("Signing out twice is harmless and leaves nothing behind the second time") + func signOutIsIdempotent() async throws { + try await withCleanAuthState { + let auth = AuthController() + establishLocalSession(auth) + auth.signOut() + auth.signOut() + + #expect(auth.state == .signedOut) + #expect(keychainRead() == nil) + #expect(auth.avatar == nil) + } + } + + @Test("Signing out with nothing stored does not crash") + func signOutFromASignedOutStateIsSafe() async throws { + try await withCleanAuthState { + let auth = AuthController() + auth.signOut() + #expect(auth.state == .signedOut) + #expect(keychainRead() == nil) + } + } + + @Test("After sign-out a fresh launch finds no session") + func noResidualSessionAfterSignOut() async throws { + try await withCleanAuthState { + let auth = AuthController() + establishLocalSession(auth) + auth.setAvatar(jpegFixture(150, 150)) + auth.signOut() + + let relaunched = AuthController() + await relaunched.restore() + #expect(relaunched.state == .signedOut) + #expect(relaunched.isStubbedSession == false) + #expect(relaunched.avatar == nil) + #expect(relaunched.displayName == nil) + #expect(relaunched.email == nil) + } + } + + @Test("A launch with an empty keychain stays signed out") + func launchWithNoStoredSession() async throws { + try await withCleanAuthState { + let auth = AuthController() + await auth.restore() + #expect(auth.state == .signedOut) + #expect(auth.isStubbedSession == false) + } + } + + @Test("A keychain item holding non-UTF8 bytes is ignored rather than trusted", + .enabled(if: keychainAvailable, keychainSkipReason)) + func corruptKeychainPayloadIsNotASession() async throws { + try await withCleanAuthState { + keychainDelete() + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrAccount as String: Storage.keychainAccount, + kSecValueData as String: Data([0xFF, 0xFE, 0x00, 0x80, 0x81]), + kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlock, + ] + SecItemAdd(query as CFDictionary, nil) + + let auth = AuthController() + await auth.restore() + + // Undecodable bytes read as no stored ID at all, so `restore()` + // returns early and the session stays closed. + #expect(auth.state == .signedOut) + #expect(auth.isStubbedSession == false) + } + } +} + +// MARK: - Credential handling + +@Suite("Credential handling · Apple's one-shot name and email", .serialized) +@MainActor +struct CredentialHandlingTests { + + @Test("A stored identity survives a later sign-in that carries no name or email") + func aLaterSignInDoesNotWipeTheStoredIdentity() async throws { + try await withCleanAuthState { + // The first authorization: Apple hands the name and email over + // once, and they are written to storage at that one opportunity. + UserDefaults.standard.set("Marie Kondo", forKey: Storage.nameKey) + UserDefaults.standard.set("marie@example.com", forKey: Storage.emailKey) + + // Every later sign-in arrives with both fields nil. The identity + // must not be cleared to match, or a returning user is nameless + // forever — Apple never offers those fields again. + let auth = AuthController() + establishLocalSession(auth) + + #expect(auth.displayName == "Marie Kondo") + #expect(auth.email == "marie@example.com") + #expect(UserDefaults.standard.string(forKey: Storage.nameKey) == "Marie Kondo") + #expect(UserDefaults.standard.string(forKey: Storage.emailKey) == "marie@example.com") + } + } + + @Test("A relaunch does not disturb the stored identity either") + func restoreDoesNotWipeTheStoredIdentity() async throws { + try await withCleanAuthState { + UserDefaults.standard.set("Marie Kondo", forKey: Storage.nameKey) + UserDefaults.standard.set("marie@example.com", forKey: Storage.emailKey) + keychainWrite(Storage.stubUserID) + + let auth = AuthController() + await auth.restore() + + #expect(auth.displayName == "Marie Kondo") + #expect(auth.email == "marie@example.com") + } + } + + @Test("Half an identity is kept as half, not discarded") + func nameWithoutEmailIsStillAName() async throws { + try await withCleanAuthState { + UserDefaults.standard.set("Marie Kondo", forKey: Storage.nameKey) + + let auth = AuthController() + #expect(auth.displayName == "Marie Kondo") + #expect(auth.email == nil) + // `AccountSheet.identityIsPartial` turns exactly this into the + // "Apple shares a name and email only the first time" line. + #expect(auth.isStubbedSession == false) + } + } + + @Test("An email without a name is kept too") + func emailWithoutNameIsStillAnEmail() async throws { + try await withCleanAuthState { + UserDefaults.standard.set("marie@example.com", forKey: Storage.emailKey) + + let auth = AuthController() + #expect(auth.email == "marie@example.com") + #expect(auth.displayName == nil) + #expect(auth.initials == nil) + } + } + + @Test("A Hide-My-Email relay address is stored verbatim") + func relayAddressIsNotRewritten() async throws { + try await withCleanAuthState { + let relay = "a1b2c3d4e5@privaterelay.appleid.com" + UserDefaults.standard.set(relay, forKey: Storage.emailKey) + #expect(AuthController().email == relay) + } + } + + @Test("No credential, name, email or user ID reaches the console") + func nothingSensitiveIsLogged() async throws { + try await withCleanAuthState { + let secrets = ["Marie Kondo", "marie.secret@example.com", Storage.stubUserID] + let picture = jpegFixture(120, 120) + + let output = await capturingConsole { + let auth = AuthController() + UserDefaults.standard.set(secrets[0], forKey: Storage.nameKey) + UserDefaults.standard.set(secrets[1], forKey: Storage.emailKey) + establishLocalSession(auth) + auth.setAvatar(picture) + await auth.restore() + _ = auth.initials + _ = auth.displayName + _ = auth.email + auth.signOut() + } + + for secret in secrets { + #expect(output.contains(secret) == false, + "a credential detail was written to stdout or stderr") + } + } + } +} + +// MARK: - Avatar storage + +@Suite("Avatar storage · where it lives and how it fails", .serialized) +@MainActor +struct AvatarStorageTests { + + @Test("The picture is written to Application Support, beside the session") + func avatarLivesInApplicationSupport() async throws { + try await withCleanAuthState { + let auth = AuthController() + auth.setAvatar(jpegFixture(300, 200)) + + let url = try #require(Storage.avatarURL) + #expect(url.lastPathComponent == "avatar.jpg") + #expect(url.path.contains("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/Library/Application Support")) + // Not Caches, which the system may evict, and not Documents, + // which is user-visible through the Files app. + #expect(url.path.contains("/Caches/") == false) + #expect(url.path.contains("/Documents/") == false) + #expect(FileManager.default.fileExists(atPath: url.path)) + #expect(auth.avatar != nil) + } + } + + @Test("A stored picture is loaded again on the next launch") + func avatarSurvivesRelaunch() async throws { + try await withCleanAuthState { + AuthController().setAvatar(jpegFixture(300, 200)) + + let relaunched = AuthController() + #expect(relaunched.avatar == nil, "nothing is read until loadAvatar runs") + relaunched.loadAvatar() + #expect(relaunched.avatar != nil) + + // And `restore()` is what calls it in the app. + let launched = AuthController() + await launched.restore() + #expect(launched.avatar != nil) + } + } + + @Test("An oversized picture is downscaled before it is kept in memory") + func oversizedPictureIsDownscaledInMemory() async throws { + try await withCleanAuthState { + let auth = AuthController() + auth.setAvatar(jpegFixture(1024, 768)) + + let image = try #require(auth.avatar) + #expect(image.size.width == 512) + #expect(image.size.height == 384) // the aspect ratio is preserved + } + } + + @Test("A picture already smaller than the cap is stored untouched") + func smallPictureIsNotUpscaled() async throws { + try await withCleanAuthState { + let auth = AuthController() + auth.setAvatar(jpegFixture(100, 80)) + + let image = try #require(auth.avatar) + #expect(image.size.width == 100) + #expect(image.size.height == 80) + } + } + + @Test("FINDING (low): the file on disk is the device scale larger than the 512pt cap") + func writtenFileIsLargerThanTheCapSuggests() async throws { + try await withCleanAuthState { + let auth = AuthController() + auth.setAvatar(jpegFixture(1024, 768)) + + let url = try #require(Storage.avatarURL) + let data = try Data(contentsOf: url) + let written = try #require(UIImage(data: data)) + + // FINDING (low): AuthController.swift:88 — `downscale` renders + // through a `UIGraphicsImageRenderer` with no explicit format, so + // it inherits the device scale. `scaled.size` is 512pt as + // intended, but `jpegData` writes 512 × scale *pixels*: 1536 px on + // a 3x phone, nine times the pixel count the comment at :68-69 + // sets out to avoid. Visually harmless; the file is simply much + // bigger than the code says it is. A format with `scale = 1` + // would fix it. + #expect(written.size.width == 512 * rendererScale) + if rendererScale > 1 { + #expect(written.size.width > 512) + #expect(data.count > 0) + } + } + } + + @Test("A file that is not an image loads as no avatar instead of crashing", + arguments: ["not an image at all", + "", + "\u{0}\u{1}\u{2}\u{3}", + "GIF89a"]) + func corruptAvatarFileLoadsAsNil(_ junk: String) async throws { + try await withCleanAuthState { + let url = try #require(Storage.avatarURL) + try Data(junk.utf8).write(to: url, options: .atomic) + + let auth = AuthController() + auth.loadAvatar() + #expect(auth.avatar == nil) + + // And the launch path walks over it without trouble either. + await auth.restore() + #expect(auth.avatar == nil) + } + } + + @Test("A truncated JPEG does not crash the loader") + func truncatedJpegIsSurvivable() async throws { + try await withCleanAuthState { + let full = jpegFixture(400, 400) + let url = try #require(Storage.avatarURL) + try full.prefix(full.count / 3).write(to: url, options: .atomic) + + let auth = AuthController() + auth.loadAvatar() + // UIImage may decode a partial JPEG or refuse it — either is + // acceptable, a crash is not. Whichever it does, it must do it + // consistently: a loader that returned an image once and nil the + // next time would make the header avatar flicker between a picture + // and a placeholder across launches. + let first = auth.avatar != nil + let second = AuthController() + second.loadAvatar() + #expect((second.avatar != nil) == first) + + // The truncated file is really there — otherwise the removal + // assertion below would pass against nothing. + #expect(FileManager.default.fileExists(atPath: url.path)) + auth.clearAvatar() + #expect(auth.avatar == nil) + #expect(FileManager.default.fileExists(atPath: url.path) == false) + } + } + + @Test("An empty avatar file loads as no avatar") + func emptyAvatarFileIsNil() async throws { + try await withCleanAuthState { + let url = try #require(Storage.avatarURL) + try Data().write(to: url, options: .atomic) + + let auth = AuthController() + auth.loadAvatar() + #expect(auth.avatar == nil) + } + } + + @Test("Loading with no file at all is not an error") + func missingAvatarFileIsNil() async throws { + try await withCleanAuthState { + let auth = AuthController() + auth.loadAvatar() + #expect(auth.avatar == nil) + } + } + + @Test("Setting a non-image keeps the picture that was already there") + func settingJunkDoesNotDestroyTheStoredPicture() async throws { + try await withCleanAuthState { + let auth = AuthController() + auth.setAvatar(jpegFixture(200, 200)) + let url = try #require(Storage.avatarURL) + let good = try Data(contentsOf: url) + + auth.setAvatar(Data("this is not a picture".utf8)) + auth.setAvatar(Data()) + + // Nothing was written, so the previous picture is intact. The + // failure is silent, which is a UI question rather than a + // security one. + #expect(try Data(contentsOf: url) == good) + #expect(auth.avatar != nil) + } + } + + @Test("Clearing removes the file, and clearing again is harmless") + func clearAvatarDeletesTheFile() async throws { + try await withCleanAuthState { + let auth = AuthController() + auth.setAvatar(jpegFixture(200, 200)) + let url = try #require(Storage.avatarURL) + #expect(FileManager.default.fileExists(atPath: url.path)) + + auth.clearAvatar() + #expect(auth.avatar == nil) + #expect(FileManager.default.fileExists(atPath: url.path) == false) + + auth.clearAvatar() + #expect(auth.avatar == nil) + } + } + + @Test("Replacing a picture leaves one file, holding the newer image") + func replacingAPictureOverwritesInPlace() async throws { + try await withCleanAuthState { + let auth = AuthController() + auth.setAvatar(jpegFixture(200, 200)) + auth.setAvatar(jpegFixture(400, 100)) + + let url = try #require(Storage.avatarURL) + let stored = try #require(UIImage(data: try Data(contentsOf: url))) + #expect(stored.size.width > stored.size.height, "the second picture is the wide one") + + let directory = url.deletingLastPathComponent() + let jpegs = try FileManager.default + .contentsOfDirectory(atPath: directory.path) + .filter { $0.hasSuffix(".jpg") } + .sorted() + #expect(jpegs == ["avatar.jpg"], "no orphaned copies accumulate") + } + } +} + +// MARK: - Initials + +/// `initials` renders inside `AvatarView`, on the header and on the account +/// sheet. A trap here is a crash on a screen the user reaches by tapping their +/// own face, so the inputs below are deliberately unpleasant. +@Suite("Initials · derived from whatever name is on file", .serialized) +@MainActor +struct InitialsTests { + + private func initials(for name: String?) -> String? { + if let name { UserDefaults.standard.set(name, forKey: Storage.nameKey) } + else { UserDefaults.standard.removeObject(forKey: Storage.nameKey) } + return AuthController().initials + } + + @Test("Two names give two letters, uppercased") + func twoNames() async throws { + try await withCleanAuthState { + #expect(initials(for: "Marie Kondo") == "MK") + #expect(initials(for: "marie kondo") == "MK") + } + } + + @Test("A single name gives one letter") + func singleName() async throws { + try await withCleanAuthState { + #expect(initials(for: "Marie") == "M") + #expect(initials(for: "x") == "X") + } + } + + @Test("More than two names still give two letters — the first two") + func manyNames() async throws { + try await withCleanAuthState { + #expect(initials(for: "Jean Baptiste Grenouille de la Fontaine") == "JB") + #expect(initials(for: "a b c d e f g h") == "AB") + } + } + + @Test("Runs of spaces do not produce empty initials") + func repeatedAndSurroundingSpaces() async throws { + try await withCleanAuthState { + #expect(initials(for: "Marie Kondo") == "MK") + #expect(initials(for: " Marie Kondo") == "MK") + #expect(initials(for: "Marie Kondo ") == "MK") + #expect(initials(for: " Marie Kondo ") == "MK") + } + } + + @Test("No name on file means no initials, never invented ones") + func noName() async throws { + try await withCleanAuthState { + // The design's "MK" was a mockup literal; a real nameless account + // gets the neutral person symbol instead. + #expect(initials(for: nil) == nil) + #expect(initials(for: "") == nil) + #expect(initials(for: " ") == nil) + #expect(initials(for: " ") == nil) + } + } + + @Test("Non-Latin scripts come through as their own letters", + arguments: zip(["Ольга Смирнова", "梅田 由紀", "김 민준", + "Δημήτρης Παπαδόπουλος", "אורי לוי", + "Ünsal Çetin", "Ægir Þórsson"], + ["ОС", "梅由", "김민", "ΔΠ", "אל", "ÜÇ", "ÆÞ"])) + func nonLatinNames(_ name: String, _ expected: String) async throws { + try await withCleanAuthState { + #expect(initials(for: name) == expected) + } + } + + @Test("Right-to-left text yields two characters and does not trap") + func arabicName() async throws { + try await withCleanAuthState { + // Not a rendering claim — only that two characters come out and + // nothing traps on the bidi run. + #expect(initials(for: "أحمد الحسن")?.count == 2) + } + } + + @Test("Emoji and combining sequences do not crash and stay one grapheme each", + arguments: ["🍇 Vigneron", + "👨‍👩‍👧‍👦 Family", + "🇫🇷 Domaine", + "e\u{0301}mile Peynaud", + "🍷🍇 🥂"]) + func emojiNames(_ name: String) async throws { + try await withCleanAuthState { + let result = initials(for: name) + #expect(result != nil) + #expect((result?.count ?? 0) <= 2, "at most one grapheme per name part") + } + } + + @Test("A very long name is still two letters and does not hang") + func absurdlyLongName() async throws { + try await withCleanAuthState { + #expect(initials(for: String(repeating: "Chateau ", count: 5_000)) == "CC") + #expect(initials(for: String(repeating: "x", count: 10_000)) == "X") + } + } + + @Test("FINDING (low): whitespace that is not an ASCII space becomes an invisible initial", + arguments: ["\t", "\n", "\u{00A0}", "\u{2007}", "\t\n"]) + func nonAsciiWhitespaceProducesABlankInitial(_ name: String) async throws { + try await withCleanAuthState { + // FINDING (low): AuthController.swift:99-100 — the name is split on + // the literal " " only, and the emptiness test is + // `letters.isEmpty`, so a name made of a tab, a newline or a + // non-breaking space produces a non-nil string holding one + // whitespace character. `AvatarView` (Views/Components/Avatar.swift:16) + // checks `!initials.isEmpty`, which that passes, so the avatar + // draws an empty tinted circle instead of falling back to the + // neutral person symbol. Splitting on `.whitespacesAndNewlines` + // and rejecting a blank result would close it. + let result = initials(for: name) + #expect(result != nil, "asserted as it is: whitespace survives as an initial") + #expect(result?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == true) + } + } + + @Test("FINDING (low): a name beginning with ß yields three characters, not two") + func sharpSUppercasesToTwoLetters() async throws { + try await withCleanAuthState { + // FINDING (low): AuthController.swift:99-100 — `prefix(2)` caps the + // number of *source* letters, then `.uppercased()` runs on the + // joined string. German ß uppercases to "SS", so two name parts + // can produce a three- or four-character initial inside a fixed + // 32pt circle sized for two. + #expect(initials(for: "ßeta Gamma") == "SSG") + #expect(initials(for: "ßruno ßauer") == "SSSS") + } + } + + @Test("The initials follow the stored name, and disappear with it") + func initialsTrackTheStoredName() async throws { + try await withCleanAuthState { + let auth = AuthController() + UserDefaults.standard.set("Marie Kondo", forKey: Storage.nameKey) + #expect(auth.initials == "MK") + + auth.signOut() + #expect(auth.initials == nil, "no initials survive sign-out") + } + } +} + +// MARK: - Shipped configuration + +/// Every permission the app actually asks for. A missing or empty usage string +/// is an automatic App Store rejection, and on device it is worse than that: +/// the API traps the moment it is called. +private let usageDescriptionKeys = [ + "NSCameraUsageDescription", + "NSMicrophoneUsageDescription", + "NSSpeechRecognitionUsageDescription", + "NSPhotoLibraryUsageDescription", +] + +/// These read the built application bundle rather than the files in the repo: +/// a privacy manifest that is present on disk but never copied into the app is +/// exactly the failure worth catching, and reading `Vinnota/PrivacyInfo.xcprivacy` +/// would not catch it. +@Suite("Shipped configuration · what App Store review looks at") +struct ShippedConfigurationTests { + + @Test("The test really is inspecting the application bundle") + func bundleIsTheApp() throws { + let identifier = try #require(appBundle.bundleIdentifier) + #expect(identifier == "com.vinnota.cellarbook") + #expect(appBundle.bundlePath.hasSuffix(".app")) + } + + // MARK: PrivacyInfo.xcprivacy + + private func privacyManifest() throws -> [String: Any] { + let url = try #require(appBundle.url(forResource: "PrivacyInfo", withExtension: "xcprivacy"), + "PrivacyInfo.xcprivacy is not in the built app bundle") + let data = try Data(contentsOf: url) + let plist = try PropertyListSerialization.propertyList(from: data, format: nil) + return try #require(plist as? [String: Any], "the privacy manifest is not a plist dictionary") + } + + @Test("The privacy manifest ships in the bundle and parses as a plist") + func privacyManifestExistsAndParses() throws { + #expect(try privacyManifest().isEmpty == false) + } + + @Test("The manifest declares no tracking, and no tracking domains to go with it") + func privacyManifestDeclaresNoTracking() throws { + let manifest = try privacyManifest() + + let tracking = try #require(manifest["NSPrivacyTracking"] as? Bool, + "NSPrivacyTracking is missing or is not a boolean") + #expect(tracking == false) + + // Apple rejects a manifest that claims no tracking while listing + // domains, so the array must be present and empty. + let domains = try #require(manifest["NSPrivacyTrackingDomains"] as? [String], + "NSPrivacyTrackingDomains is missing") + #expect(domains.isEmpty) + } + + @Test("Every required top-level key is present", + arguments: ["NSPrivacyTracking", + "NSPrivacyTrackingDomains", + "NSPrivacyCollectedDataTypes", + "NSPrivacyAccessedAPITypes"]) + func privacyManifestHasTheRequiredKeys(_ key: String) throws { + #expect(try privacyManifest()[key] != nil, "the privacy manifest is missing \(key)") + } + + @Test("Each collected data type is fully described") + func collectedDataTypesAreWellFormed() throws { + let types = try #require(try privacyManifest()["NSPrivacyCollectedDataTypes"] + as? [[String: Any]]) + #expect(types.isEmpty == false) + + for entry in types { + let name = try #require(entry["NSPrivacyCollectedDataType"] as? String) + #expect(name.hasPrefix("NSPrivacyCollectedDataType")) + #expect(entry["NSPrivacyCollectedDataTypeLinked"] as? Bool != nil, + "\(name) does not say whether it is linked to the user") + // Nothing may be declared as used for tracking in a manifest that + // says the app does not track. + #expect(entry["NSPrivacyCollectedDataTypeTracking"] as? Bool == false, + "\(name) claims tracking in a manifest that declares none") + let purposes = try #require(entry["NSPrivacyCollectedDataTypePurposes"] as? [String]) + #expect(purposes.isEmpty == false, "\(name) declares no purpose") + } + } + + @Test("The identity the app actually stores is declared", + arguments: ["NSPrivacyCollectedDataTypeName", + "NSPrivacyCollectedDataTypeEmailAddress", + "NSPrivacyCollectedDataTypeUserID"]) + func collectedDataTypesCoverTheStoredIdentity(_ expected: String) throws { + // AuthController writes a display name, an email address and an Apple + // user ID. All three have to appear here. + let types = try #require(try privacyManifest()["NSPrivacyCollectedDataTypes"] + as? [[String: Any]]) + let declared = Set(types.compactMap { $0["NSPrivacyCollectedDataType"] as? String }) + #expect(declared.contains(expected)) + } + + @Test("Each accessed API category carries at least one reason code") + func accessedAPITypesCarryReasons() throws { + let apis = try #require(try privacyManifest()["NSPrivacyAccessedAPITypes"] + as? [[String: Any]]) + #expect(apis.isEmpty == false) + + for entry in apis { + let category = try #require(entry["NSPrivacyAccessedAPIType"] as? String) + #expect(category.hasPrefix("NSPrivacyAccessedAPICategory")) + let reasons = try #require(entry["NSPrivacyAccessedAPITypeReasons"] as? [String]) + #expect(reasons.isEmpty == false, "\(category) declares no reason code") + for reason in reasons { #expect(reason.isEmpty == false) } + } + } + + @Test("UserDefaults, which holds the account identity, is declared as an accessed API") + func userDefaultsAccessIsDeclared() throws { + let apis = try #require(try privacyManifest()["NSPrivacyAccessedAPITypes"] + as? [[String: Any]]) + let categories = Set(apis.compactMap { $0["NSPrivacyAccessedAPIType"] as? String }) + #expect(categories.contains("NSPrivacyAccessedAPICategoryUserDefaults")) + } + + // MARK: Info.plist + + @Test("Every permission the app requests has a usage string", + arguments: usageDescriptionKeys) + func usageDescriptionIsPresentAndUseful(_ key: String) throws { + let value = try #require(appBundle.object(forInfoDictionaryKey: key) as? String, + "\(key) is missing from the shipped Info.plist") + + #expect(value.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == false, + "\(key) is empty") + // A one-word string passes review about as often as a missing one. + #expect(value.count >= 20, "\(key) does not explain anything") + #expect(value.hasSuffix("."), "\(key) is not written as a sentence") + // An unexpanded build setting ships the literal "$(…)" to the user. + #expect(value.contains("$(") == false, "\(key) contains an unexpanded build setting") + #expect(value.localizedCaseInsensitiveContains("todo") == false) + } + + @Test("The usage strings are distinct, so each prompt explains its own permission") + func usageDescriptionsAreNotCopyPasted() { + let values = usageDescriptionKeys.compactMap { + appBundle.object(forInfoDictionaryKey: $0) as? String + } + #expect(values.count == usageDescriptionKeys.count) + #expect(Set(values).count == values.count) + } + + @Test("The speech prompt is honest about where dictation is processed") + func speechDescriptionMentionsApple() throws { + // `SpeechTranscriber` falls back to Apple's servers when no on-device + // model exists, and this dialog is the only place the user is told. + let value = try #require( + appBundle.object(forInfoDictionaryKey: "NSSpeechRecognitionUsageDescription") as? String) + #expect(value.contains("Apple")) + } + + @Test("Bundle identity and version are real values, not unexpanded settings", + arguments: ["CFBundleIdentifier", "CFBundleShortVersionString", + "CFBundleVersion", "CFBundleName", "CFBundleExecutable"]) + func bundleIdentityIsResolved(_ key: String) throws { + let value = try #require(appBundle.object(forInfoDictionaryKey: key) as? String, + "\(key) is missing") + #expect(value.isEmpty == false) + #expect(value.contains("$(") == false, "\(key) was never expanded") + } +} From 35b97b6676390b8116688170052a3332d68a9b24 Mon Sep 17 00:00:00 2001 From: Sujoy Das Date: Sat, 5 Sep 2026 22:42:28 +0300 Subject: [PATCH 6/8] ci: run the keychain-backed auth tests and fail if any test skips --- .github/workflows/ci.yml | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 32090f1..c92a84d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -125,6 +125,12 @@ jobs: echo "Using: $name" echo "name=$name" >> "$GITHUB_OUTPUT" + # Deliberately does NOT pass CODE_SIGNING_ALLOWED=NO. An unsigned app has + # no application-identifier entitlement, so the simulator keychain answers + # errSecMissingEntitlement and the session-persistence tests skip instead + # of running — the auth suite would report green while never touching the + # keychain. Simulator builds sign ad-hoc without a team, so this works on + # a bare runner. - name: Test if: steps.probe.outputs.found == 'true' run: | @@ -133,5 +139,16 @@ jobs: -scheme "$SCHEME" \ -destination "platform=iOS Simulator,name=${{ steps.sim.outputs.name }}" \ -derivedDataPath build/dd \ - CODE_SIGNING_ALLOWED=NO \ - test + test 2>&1 | tee test.log + + # A suite that skips its security tests must not read as a pass. + - name: Fail if security tests were skipped + if: steps.probe.outputs.found == 'true' + run: | + skipped=$(grep -cE "skipped:" test.log || true) + echo "skipped tests: $skipped" + if [ "$skipped" -ne 0 ]; then + grep -E "skipped:" test.log | sort -u + echo "::error::Tests skipped — the keychain-backed auth tests did not run" + exit 1 + fi From 6dfd1521379cc8649d8abd2daf856399880a6e42 Mon Sep 17 00:00:00 2001 From: Sujoy Das Date: Sun, 6 Sep 2026 01:14:41 +0300 Subject: [PATCH 7/8] test: make toast expiry independent of wall-clock timing so CI stops flaking --- Vinnota/Model/AppState.swift | 8 +++- VinnotaTests/PersistenceAndStateTests.swift | 50 ++++++++++++++++----- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/Vinnota/Model/AppState.swift b/Vinnota/Model/AppState.swift index 9654167..ec7d505 100644 --- a/Vinnota/Model/AppState.swift +++ b/Vinnota/Model/AppState.swift @@ -55,11 +55,15 @@ final class AppState { } /// Toasts clear themselves after 2.6s, as the design's `toast()` does. - func showToast(_ message: String) { + /// The lifetime is a parameter so tests can exercise expiry and timer + /// cancellation without waiting out the real 2.6 seconds — a test that + /// sleeps against the true duration has only a fraction of a second of + /// margin, and flakes on a loaded machine. Callers use the default. + func showToast(_ message: String, for duration: Duration = .seconds(2.6)) { toastTask?.cancel() withAnimation(.easeOut(duration: 0.2)) { toast = message } toastTask = Task { - try? await Task.sleep(for: .seconds(2.6)) + try? await Task.sleep(for: duration) guard !Task.isCancelled else { return } withAnimation(.easeOut(duration: 0.2)) { toast = nil } } diff --git a/VinnotaTests/PersistenceAndStateTests.swift b/VinnotaTests/PersistenceAndStateTests.swift index a510f95..7e79494 100644 --- a/VinnotaTests/PersistenceAndStateTests.swift +++ b/VinnotaTests/PersistenceAndStateTests.swift @@ -1015,25 +1015,53 @@ struct ToastTests { #expect(app.toast == "second") } + /// Polls rather than sleeping a fixed span: the assertion is "it becomes + /// nil", and on a contended CI runner a fixed sleep sized to the real 2.6s + /// lifetime has too little margin. This failed in CI for exactly that + /// reason, on a run that took 312s against 4s locally. + private func waitForToastToClear(_ app: AppState, + within limit: Duration = .seconds(15)) async -> Bool { + let deadline = ContinuousClock.now.advanced(by: limit) + while ContinuousClock.now < deadline { + if app.toast == nil { return true } + try? await Task.sleep(for: .milliseconds(20)) + } + return app.toast == nil + } + /// The replacement cancels the first toast's timer. Without that, the /// earlier timer would fire mid-way through the second toast and blank it. + /// + /// The two lifetimes are deliberately far apart — the first expires almost + /// at once, the second not for half a minute — so the check cannot flake on + /// a slow machine. A delayed wake makes the first timer *more* likely to + /// have fired, not less, so slowness cannot mask the bug. @Test("The replaced toast's timer does not blank its successor", .timeLimit(.minutes(1))) func replacementCancelsTheOldTimer() async throws { let app = AppState() - app.showToast("first") - try await Task.sleep(for: .seconds(2.0)) - app.showToast("second") - try await Task.sleep(for: .seconds(1.2)) - #expect(app.toast == "second", "the first timer fired at 2.6s and must have been cancelled") + app.showToast("first", for: .milliseconds(50)) + app.showToast("second", for: .seconds(30)) + try await Task.sleep(for: .seconds(1)) + #expect(app.toast == "second", + "the first toast's timer was due long ago and must have been cancelled") } - @Test("A toast clears itself after its 2.6s life", .timeLimit(.minutes(1))) + @Test("A toast clears itself when its life is up", .timeLimit(.minutes(1))) func toastExpires() async throws { + let app = AppState() + app.showToast("Added to the book", for: .milliseconds(50)) + #expect(app.toast != nil, "the toast is visible before its timer fires") + #expect(await waitForToastToClear(app), "the toast never cleared itself") + } + + /// The shipped lifetime is still the design's 2.6s — the parameter above is + /// a test seam, not a behaviour change, and this pins the default. + @Test("The default lifetime is unchanged") + func defaultLifetimeIsTwoPointSix() async throws { let app = AppState() app.showToast("Added to the book") - #expect(app.toast != nil) - try await Task.sleep(for: .seconds(3.4)) - #expect(app.toast == nil) + try await Task.sleep(for: .seconds(1)) + #expect(app.toast != nil, "a 2.6s toast is still up after 1s") } @Test("reset() clears a live toast at once") @@ -1128,7 +1156,7 @@ struct ResetTests { @Test("FINDING: reset() leaves `editing` pointing at the previous bottle") func resetDoesNotClearEditing() { let app = dirtied() - let stale = try? #require(app.editing) + let stale = app.editing app.reset() #expect(app.editing != nil, "documented current behaviour") #expect(app.editing === stale, "still the pre-sign-out bottle") @@ -1145,7 +1173,7 @@ struct ResetTests { @Test("FINDING: a reset state still reads as 'editing' to the Review screen") func staleEditingSurvivesAFreshForm() { let app = dirtied() - let victim = try? #require(app.editing) + let victim = app.editing app.reset() // What ScanView does on the way to the review screen. From 4583400af078a09389e088c929c68c605af56c52 Mon Sep 17 00:00:00 2001 From: Sujoy Das Date: Sun, 6 Sep 2026 01:14:41 +0300 Subject: [PATCH 8/8] ci: fail on Swift warnings in the test target as well as the app --- .github/workflows/ci.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c92a84d..16806fe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -141,6 +141,18 @@ jobs: -derivedDataPath build/dd \ test 2>&1 | tee test.log + # The build job's gate only compiles the app target, so warnings in the + # test target were invisible to CI until a clean build surfaced them. + - name: Fail on Swift warnings in the test target + if: steps.probe.outputs.found == 'true' + run: | + count=$(grep -cE "\.swift.*warning:" test.log || true) + echo "Swift warnings: $count" + if [ "$count" -ne 0 ]; then + grep -E "\.swift.*warning:" test.log | sort -u + exit 1 + fi + # A suite that skips its security tests must not read as a pass. - name: Fail if security tests were skipped if: steps.probe.outputs.found == 'true'