diff --git a/projects/ios-readplace/Makefile b/projects/ios-readplace/Makefile index efdcaed47..c95acb3f3 100644 --- a/projects/ios-readplace/Makefile +++ b/projects/ios-readplace/Makefile @@ -26,9 +26,13 @@ open: generate # someone builds `make ipa-staging` by hand. SIM ?= platform=iOS Simulator,name=$(shell xcrun simctl list devices available | grep -m1 -oE 'iPhone [0-9]+') SIM_DEVICE = $(shell printf '%s' '$(SIM)' | sed -E 's/.*=//') +COVERAGE_RESULT = build/TestResults.xcresult test: generate xcrun simctl bootstatus '$(SIM_DEVICE)' -b - xcodebuild test -project Readplace.xcodeproj -scheme Readplace -destination '$(SIM)' + rm -rf $(COVERAGE_RESULT) + xcodebuild test -project Readplace.xcodeproj -scheme Readplace -destination '$(SIM)' \ + -enableCodeCoverage YES -resultBundlePath $(COVERAGE_RESULT) + python3 scripts/check-coverage.py $(COVERAGE_RESULT) $(MAKE) test-staging SIM='$(SIM)' # Compile the STAGING condition and smoke-test it. The full suite can't run under diff --git a/projects/ios-readplace/README.md b/projects/ios-readplace/README.md index aeafed724..e5ca66b69 100644 --- a/projects/ios-readplace/README.md +++ b/projects/ios-readplace/README.md @@ -234,8 +234,12 @@ You need a Mac with **Xcode 15+** and an Apple ID (a free personal team is fine) `make test` (or `Cmd+U` in Xcode) runs the XCTest suite. The network is stubbed with a `URLProtocol`, so tests exercise the real client logic — headers, bodies, -redirects, retries — without a server. Coverage focuses on boundaries and edge -cases: +redirects, retries — without a server. `make test` then enforces a per-file +line-coverage gate (`scripts/check-coverage.py` against +`scripts/coverage-baseline.json`): pure SwiftUI views and WebKit/OS-boundary glue +are excluded, and every other source file must stay at or above its recorded +floor — a ratchet the logic files are walked toward 100%. Coverage focuses on +boundaries and edge cases: - **Siren decoding**: rich vs. minimal entities, JSON `null` image/`readAt`, read-state from `status`/`readAt`, title fallback to URL, entities without diff --git a/projects/ios-readplace/ShareExtension/ShareViewController.swift b/projects/ios-readplace/ShareExtension/ShareViewController.swift index 70800114e..cc01f8155 100644 --- a/projects/ios-readplace/ShareExtension/ShareViewController.swift +++ b/projects/ios-readplace/ShareExtension/ShareViewController.swift @@ -29,24 +29,16 @@ final class ShareViewController: UIViewController { let sharedPdf: (() async -> Data?)? = shared?.pdfProvider.map { provider in { await ShareURLExtractor.loadPDFData(provider) } } - switch await saver.run(url: shared?.url, fallbackTitle: shared?.title, sharedPdf: sharedPdf) { - case .savedWithContent: - finish(message: "Saved with content", symbol: "checkmark.circle.fill", tint: .brandSuccess) - case .savedLinkOnly: - finish(message: "Saved (link only)", symbol: "checkmark.circle.fill", tint: .brandSuccess) - case .notLoggedIn: - finish(message: "Open Readplace and sign in first.", - symbol: "person.crop.circle.badge.exclamationmark", tint: .brandWarning) - case .noLink: - finish(message: "No link found to save.", symbol: "link", tint: .brandWarning) - case .noSaveAction: - finish(message: "The server offered no save action.", - symbol: "exclamationmark.triangle.fill", tint: .brandError) - case .refused(let messages): - finish(message: messages.map(\.plainText).joined(separator: "\n"), symbol: "lock.fill", - tint: messages.contains { $0.kind == .error } ? .brandError : .brandWarning) - case .failed(let message): - finish(message: message, symbol: "exclamationmark.triangle.fill", tint: .brandError) + let outcome = await saver.run(url: shared?.url, fallbackTitle: shared?.title, sharedPdf: sharedPdf) + let status = ShareStatusPresentation(outcome: outcome) + finish(message: status.message, symbol: status.symbol, tint: uiColor(for: status.tone)) + } + + private func uiColor(for tone: ShareStatusTone) -> UIColor { + switch tone { + case .success: return .brandSuccess + case .warning: return .brandWarning + case .error: return .brandError } } diff --git a/projects/ios-readplace/Shared/ShareStatusPresentation.swift b/projects/ios-readplace/Shared/ShareStatusPresentation.swift new file mode 100644 index 000000000..4b4847c82 --- /dev/null +++ b/projects/ios-readplace/Shared/ShareStatusPresentation.swift @@ -0,0 +1,54 @@ +import Foundation + +/// The visual tone of a share-sheet status, mapped to a brand colour at the +/// UIKit boundary. Kept UIKit-free so the outcome→status mapping stays a pure, +/// testable value. +enum ShareStatusTone: Equatable { + case success + case warning + case error +} + +/// What the share-sheet shell shows for a save outcome: the message text, an SF +/// Symbol name, and a tone. Lifted out of `ShareViewController` so the whole +/// mapping — including joining a refusal's messages and choosing error vs. +/// warning from their kinds — is a pure value the tests exercise directly; the +/// controller only paints it and maps the tone to a `UIColor`. +struct ShareStatusPresentation: Equatable { + let message: String + let symbol: String + let tone: ShareStatusTone + + init(outcome: SaveSharedOutcome) { + switch outcome { + case .savedWithContent: + message = "Saved with content" + symbol = "checkmark.circle.fill" + tone = .success + case .savedLinkOnly: + message = "Saved (link only)" + symbol = "checkmark.circle.fill" + tone = .success + case .notLoggedIn: + message = "Open Readplace and sign in first." + symbol = "person.crop.circle.badge.exclamationmark" + tone = .warning + case .noLink: + message = "No link found to save." + symbol = "link" + tone = .warning + case .noSaveAction: + message = "The server offered no save action." + symbol = "exclamationmark.triangle.fill" + tone = .error + case .refused(let messages): + message = messages.map(\.plainText).joined(separator: "\n") + symbol = "lock.fill" + tone = messages.contains { $0.kind == .error } ? .error : .warning + case .failed(let failureMessage): + message = failureMessage + symbol = "exclamationmark.triangle.fill" + tone = .error + } + } +} diff --git a/projects/ios-readplace/ShareExtension/ShareURLExtractor.swift b/projects/ios-readplace/Shared/ShareURLExtractor.swift similarity index 92% rename from projects/ios-readplace/ShareExtension/ShareURLExtractor.swift rename to projects/ios-readplace/Shared/ShareURLExtractor.swift index d79696374..f0638e8ef 100644 --- a/projects/ios-readplace/ShareExtension/ShareURLExtractor.swift +++ b/projects/ios-readplace/Shared/ShareURLExtractor.swift @@ -19,7 +19,13 @@ enum ShareURLExtractor { static func extract(from context: NSExtensionContext?) async -> Shared? { guard let items = context?.inputItems as? [NSExtensionItem] else { return nil } + return await extract(from: items) + } + /// The extraction core, taking the item list directly so a test can drive it + /// with fabricated `NSItemProvider`s instead of standing up a live extension + /// context (which no test can construct). + static func extract(from items: [NSExtensionItem]) async -> Shared? { // Scan every item before deciding: a host can deliver the PDF file and // the web URL as separate extension items, and the first item alone // would look like a URL-less share. diff --git a/projects/ios-readplace/Tests/ShareStatusPresentationTests.swift b/projects/ios-readplace/Tests/ShareStatusPresentationTests.swift new file mode 100644 index 000000000..a6b9477c0 --- /dev/null +++ b/projects/ios-readplace/Tests/ShareStatusPresentationTests.swift @@ -0,0 +1,64 @@ +import XCTest +@testable import Readplace + +final class ShareStatusPresentationTests: XCTestCase { + private func present(_ outcome: SaveSharedOutcome) -> ShareStatusPresentation { + ShareStatusPresentation(outcome: outcome) + } + + private func message(type: String, body: String = "x") -> ServerMessage { + ServerMessage(type: type, content: ServerMessage.Content(type: "text/html", body: body)) + } + + func testSavedWithContentIsSuccess() { + let status = present(.savedWithContent) + XCTAssertEqual(status.message, "Saved with content") + XCTAssertEqual(status.symbol, "checkmark.circle.fill") + XCTAssertEqual(status.tone, .success) + } + + func testSavedLinkOnlyIsSuccess() { + let status = present(.savedLinkOnly) + XCTAssertEqual(status.message, "Saved (link only)") + XCTAssertEqual(status.tone, .success) + } + + func testNotLoggedInIsWarning() { + let status = present(.notLoggedIn) + XCTAssertEqual(status.message, "Open Readplace and sign in first.") + XCTAssertEqual(status.symbol, "person.crop.circle.badge.exclamationmark") + XCTAssertEqual(status.tone, .warning) + } + + func testNoLinkIsWarning() { + let status = present(.noLink) + XCTAssertEqual(status.message, "No link found to save.") + XCTAssertEqual(status.symbol, "link") + XCTAssertEqual(status.tone, .warning) + } + + func testNoSaveActionIsError() { + let status = present(.noSaveAction) + XCTAssertEqual(status.message, "The server offered no save action.") + XCTAssertEqual(status.tone, .error) + } + + func testRefusedJoinsMessagesAndIsWarningWhenNoneAreErrors() { + let status = present(.refused([message(type: "warning", body: "one"), message(type: "warning", body: "two")])) + XCTAssertEqual(status.message, "one\ntwo") + XCTAssertEqual(status.symbol, "lock.fill") + XCTAssertEqual(status.tone, .warning) + } + + func testRefusedIsErrorWhenAnyMessageIsAnError() { + let status = present(.refused([message(type: "warning"), message(type: "error")])) + XCTAssertEqual(status.tone, .error) + } + + func testFailedCarriesTheServerMessageAsError() { + let status = present(.failed("Something broke")) + XCTAssertEqual(status.message, "Something broke") + XCTAssertEqual(status.symbol, "exclamationmark.triangle.fill") + XCTAssertEqual(status.tone, .error) + } +} diff --git a/projects/ios-readplace/Tests/ShareURLExtractorTests.swift b/projects/ios-readplace/Tests/ShareURLExtractorTests.swift new file mode 100644 index 000000000..258166e49 --- /dev/null +++ b/projects/ios-readplace/Tests/ShareURLExtractorTests.swift @@ -0,0 +1,112 @@ +import UniformTypeIdentifiers +import XCTest +@testable import Readplace + +@MainActor +final class ShareURLExtractorTests: XCTestCase { + private func item(_ providers: [NSItemProvider], title: String? = nil) -> NSExtensionItem { + let item = NSExtensionItem() + item.attachments = providers + if let title { item.attributedContentText = NSAttributedString(string: title) } + return item + } + + private func urlProvider(_ string: String) -> NSItemProvider { + NSItemProvider(item: URL(string: string)! as NSURL, typeIdentifier: UTType.url.identifier) + } + + private func textProvider(_ text: String) -> NSItemProvider { + NSItemProvider(item: text as NSString, typeIdentifier: UTType.plainText.identifier) + } + + /// A PDF item provider backed by a real temp file so `loadFileRepresentation` + /// and the size check run. A `truncate`d sparse file gives a large logical size + /// without writing megabytes. + private func pdfProvider(logicalSize: Int = 64, name: String = "doc.pdf") throws -> NSItemProvider { + let dir = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString, isDirectory: true) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + let url = dir.appendingPathComponent(name) + var bytes = Data("%PDF-1.4\n".utf8) + if logicalSize > bytes.count { bytes.append(Data(count: min(logicalSize - bytes.count, 4096))) } + try bytes.write(to: url) + if logicalSize > bytes.count { + let handle = try FileHandle(forWritingTo: url) + try handle.truncate(atOffset: UInt64(logicalSize)) + try handle.close() + } + let provider = try XCTUnwrap(NSItemProvider(contentsOf: url)) + provider.suggestedName = name + return provider + } + + func testExtractsAWebURL() async { + let shared = await ShareURLExtractor.extract(from: [item([urlProvider("https://example.com/post")])]) + XCTAssertEqual(shared?.url?.absoluteString, "https://example.com/post") + XCTAssertNil(shared?.pdfProvider) + } + + func testExtractsAURLFromPlainText() async { + let shared = await ShareURLExtractor.extract(from: [item([textProvider("read this https://example.com/p ok")])]) + XCTAssertEqual(shared?.url?.absoluteString, "https://example.com/p") + } + + func testExtractsURLDeliveredAsData() async { + let data = URL(string: "https://example.com/d")!.dataRepresentation + let provider = NSItemProvider(item: data as NSData, typeIdentifier: UTType.url.identifier) + let shared = await ShareURLExtractor.extract(from: [item([provider])]) + XCTAssertEqual(shared?.url?.absoluteString, "https://example.com/d") + } + + func testExtractsURLDeliveredAsString() async { + let provider = NSItemProvider(item: "https://example.com/s" as NSString, typeIdentifier: UTType.url.identifier) + let shared = await ShareURLExtractor.extract(from: [item([provider])]) + XCTAssertEqual(shared?.url?.absoluteString, "https://example.com/s") + } + + func testIgnoresANonWebURL() async { + let shared = await ShareURLExtractor.extract(from: [item([urlProvider("mailto:a@b.com")])]) + XCTAssertNil(shared, "a mailto link is not a web URL and there is no PDF, so nothing is saveable") + } + + func testFindsURLAndPDFAcrossSeparateItems() async throws { + let shared = await ShareURLExtractor.extract(from: [ + item([urlProvider("https://example.com/a")]), + item([try pdfProvider()]), + ]) + XCTAssertEqual(shared?.url?.absoluteString, "https://example.com/a") + XCTAssertNotNil(shared?.pdfProvider) + } + + func testTitleComesFromAttributedContentText() async { + let shared = await ShareURLExtractor.extract(from: [item([urlProvider("https://example.com/a")], title: "My Title")]) + XCTAssertEqual(shared?.title, "My Title") + } + + func testTitleFallsBackToPdfSuggestedNameWhenNoContentText() async throws { + let shared = await ShareURLExtractor.extract(from: [item([try pdfProvider(name: "report.pdf")])]) + XCTAssertNil(shared?.url) + XCTAssertNotNil(shared?.pdfProvider) + XCTAssertEqual(shared?.title, "report.pdf", "with no content text the title falls back to the PDF's suggested name") + } + + func testReturnsNilWhenNoURLAndNoPDF() async { + let shared = await ShareURLExtractor.extract(from: [item([textProvider("just words, no link")])]) + XCTAssertNil(shared) + } + + func testReturnsNilForNoItems() async { + let shared = await ShareURLExtractor.extract(from: []) + XCTAssertNil(shared) + } + + func testLoadsPdfDataUnderTheCeiling() async throws { + let data = await ShareURLExtractor.loadPDFData(try pdfProvider(logicalSize: 256)) + XCTAssertEqual(data?.starts(with: Data("%PDF-".utf8)), true) + } + + func testLoadPdfDataRejectsAnOversizeFile() async throws { + let oversize = try pdfProvider(logicalSize: ReadplaceAPI.defaultMaxExternalContentBytes + 1) + let data = await ShareURLExtractor.loadPDFData(oversize) + XCTAssertNil(data, "a PDF over the extension's byte ceiling is not pulled into memory") + } +} diff --git a/projects/ios-readplace/scripts/check-coverage.py b/projects/ios-readplace/scripts/check-coverage.py new file mode 100755 index 000000000..2bfd2f1a4 --- /dev/null +++ b/projects/ios-readplace/scripts/check-coverage.py @@ -0,0 +1,104 @@ +#!/usr/bin/env python3 +"""Enforce per-file line-coverage floors for the iOS app after `make test`. + +The rest of the repo enforces a 100% coverage gate; iOS had none. This reads the +`.xcresult` bundle `xcodebuild test -enableCodeCoverage YES` produced and fails +the build if any measured source file dropped below its recorded floor. + +Config (`coverage-baseline.json`, beside this script): + - `excluded`: files that are pure SwiftUI layout, UIKit shells, or WebKit / + OS-boundary glue — unit-uncoverable without UI tests. This is the Swift + analog of the repo's whole-file OS-boundary coverage exclusion. + - `floors`: the minimum whole-percent line coverage each remaining source file + must keep. A source file with neither an exclusion nor a floor must be 100%, + so a newly added logic file cannot land untested. Floors are a RATCHET: raise + them as coverage improves, never lower them. + +Uses only the system `python3` + `xcrun`, so the CI `ios-tests` job needs no +extra toolchain setup. +""" +import json +import subprocess +import sys +from pathlib import Path + +CONFIG_PATH = Path(__file__).with_name("coverage-baseline.json") + + +def load_report(xcresult: str) -> dict: + out = subprocess.run( + ["xcrun", "xccov", "view", "--report", "--json", xcresult], + capture_output=True, text=True, check=True, + ).stdout + return json.loads(out) + + +def measured_files(report: dict) -> dict[str, float]: + """Best line coverage per source basename across all non-test targets. + + A Shared file compiles into both the app and the extension target, so it + appears twice with identical coverage; keep the higher of the two. + """ + best: dict[str, float] = {} + for target in report.get("targets", []): + if "Tests" in target.get("name", ""): + continue + for f in target.get("files", []): + name = f.get("name", "") + if not name.endswith(".swift"): + continue + cov = float(f.get("lineCoverage", 0.0)) + best[name] = max(best.get(name, 0.0), cov) + return best + + +def main() -> int: + if len(sys.argv) != 2: + print("usage: check-coverage.py ", file=sys.stderr) + return 2 + + config = json.loads(CONFIG_PATH.read_text()) + excluded = config.get("excluded", {}) + floors = config.get("floors", {}) + + report = load_report(sys.argv[1]) + measured = measured_files(report) + + failures: list[str] = [] + slack: list[str] = [] + for name, cov in sorted(measured.items()): + if name in excluded: + continue + percent = round(cov * 100) + floor = floors.get(name, 100) + if percent < floor: + failures.append(f" {name}: {percent}% < floor {floor}%") + elif percent >= floor + 2: + # A 1% headroom is a deliberate buffer against minor coverage drift; + # only nudge a ratchet once a file is comfortably above its floor. + slack.append(f" {name}: {percent}% (floor {floor}%)") + + stale = sorted({n for n in list(excluded) + list(floors) if n not in measured}) + if stale: + print("warning: coverage config references files no longer measured — prune them:") + for n in stale: + print(f" {n}") + print() + + if failures: + print("iOS coverage gate FAILED — files below their floor:") + print("\n".join(failures)) + print("\nRaise the file's coverage, or (only with approval) its floor in " + f"{CONFIG_PATH.name}.") + return 1 + + print(f"iOS coverage gate passed: {len(measured) - len(excluded)} files at " + f"or above floor, {len(excluded)} excluded (view / OS-boundary).") + if slack: + print("Ratchet candidates (coverage now exceeds the floor — raise it):") + print("\n".join(slack)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/projects/ios-readplace/scripts/coverage-baseline.json b/projects/ios-readplace/scripts/coverage-baseline.json new file mode 100644 index 000000000..6529d41fe --- /dev/null +++ b/projects/ios-readplace/scripts/coverage-baseline.json @@ -0,0 +1,37 @@ +{ + "_comment": "Per-file line-coverage floors for the iOS app, enforced by check-coverage.py after `make test`. `excluded` lists files that are pure SwiftUI layout, UIKit shells, or WebKit/OS-boundary glue — unit-uncoverable without UI tests (the Swift analog of the repo's whole-file OS-boundary coverage exclusion). Every other source file must stay at or above its floor; a source file with no floor must be 100%, so a new logic file cannot land untested. Floors are a RATCHET: raise them as coverage improves (the E-series PRs walk the logic files toward 100% and move ShareURLExtractor out of `excluded`), never lower them without approval.", + "excluded": { + "WebAuthOpeners.swift": "UIApplication URL-opening glue (OS boundary)", + "WebPageView.swift": "WKWebView SwiftUI wrapper (OS boundary)", + "ReaderWebView.swift": "WKWebView + JS-bridge SwiftUI wrapper (OS boundary); the pure bridge parser is covered by ReaderBridgeTests", + "HTMLCaptor.swift": "WKWebView navigation/capture state machine (OS boundary); the pure navigationResponseDecision is covered by HTMLCaptorTests", + "AddLinkInstructionsView.swift": "pure SwiftUI layout", + "ReaderSheet.swift": "pure SwiftUI layout", + "ArticleRow.swift": "pure SwiftUI layout", + "ReadingListView.swift": "pure SwiftUI layout", + "ShareViewController.swift": "UIKit share-extension shell (OS boundary)" + }, + "floors": { + "AppConfig.swift": 100, + "BrandColor.swift": 100, + "PendingAuthStore.swift": 100, + "ToolbarRoute.swift": 100, + "ReaderNavigation.swift": 100, + "ShareStatusPresentation.swift": 100, + "ShareURLExtractor.swift": 96, + "SirenModels.swift": 99, + "WebAuthFlow.swift": 97, + "PKCE.swift": 95, + "ReadplaceAPI.swift": 94, + "URLDetection.swift": 94, + "ReadingListViewModel.swift": 93, + "SaveSharedPage.swift": 93, + "LoginView.swift": 91, + "AffordancePresentation.swift": 91, + "OAuthService.swift": 89, + "TokenStore.swift": 85, + "AppSession.swift": 79, + "ReadplaceApp.swift": 64, + "KeychainTokenStorage.swift": 57 + } +}