amadan

oficina / sheets Public

Keep workbook export names and verify native recovery flows

Paul Campbell pushed by codex b3038ce52fd0573e0714b70bff96170b021a8e02 parent ecd9503
4 files changed, +74 −19
  • internal/sheets/native_review_test.go +28 −7
  • native/REVIEW.md +27 −5
  • native/apple/Sources/SheetsKit/SheetFiles.swift +14 −7
  • native/ios/UITests/NativeUITests.swift +5 −0
diff --git a/internal/sheets/native_review_test.go b/internal/sheets/native_review_test.go
index 925f533..4bf21af 100644
--- a/internal/sheets/native_review_test.go
+++ b/internal/sheets/native_review_test.go
@@ -116,6 +116,12 @@ func TestNativeReview(t *testing.T) {
if err := os.Remove(credentialPath); err != nil && !os.IsNotExist(err) {
t.Errorf("remove review credential: %v", err)
}
+ staged, _ := filepath.Glob(filepath.Join(strings.TrimSpace(string(current)), "tmp", "oficina-sheets-export-*"))
+ for _, file := range staged {
+ if err := os.RemoveAll(file); err != nil {
+ t.Errorf("remove isolated export artifact: %v", err)
+ }
+ }
}()
defer exec.Command("xcrun", "simctl", "terminate", simulator, bundle).Run()
if os.Getenv("NATIVE_REVIEW_UI_TESTS") != "" {
@@ -124,6 +130,10 @@ func TestNativeReview(t *testing.T) {
defer cancel()
command := exec.CommandContext(commandContext, "xcodebuild", "-project", "native/ios/SheetsPhone.xcodeproj", "-scheme", "SheetsReviewPhone", "-destination", "platform=iOS Simulator,id="+simulator, "-parallel-testing-enabled", "NO", "-derivedDataPath", "native/build/review-ios", "CODE_SIGNING_ALLOWED=NO", "-resultBundlePath", resultBundle, "test-without-building")
command.Dir = "../.."
+ only := os.Getenv("NATIVE_REVIEW_UI_ONLY")
+ if only != "" {
+ command.Args = append(command.Args, "-only-testing:SheetsPhoneUITests/"+only)
+ }
logPath := filepath.Join(os.TempDir(), "sheets-native-ui-"+simulator+".log")
logFile, err := os.OpenFile(logPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
if err != nil {
@@ -135,12 +145,23 @@ func TestNativeReview(t *testing.T) {
if err := command.Run(); err != nil {
t.Fatal("Native UI tests failed; see", logPath, err)
}
- verifyReviewFormula(t, a, credential, seed.ID)
- if !refusedSyntheticEdit.Load() || !requestedExport.Load() {
- t.Fatal("Native retry and export controls did not reach their isolated server paths")
+ if only == "" {
+ verifyReviewFormula(t, a, credential, seed.ID, true, true)
+ if !refusedSyntheticEdit.Load() || !requestedExport.Load() {
+ t.Fatal("Native retry and export controls did not reach their isolated server paths")
+ }
+ } else if strings.Contains(only, "CreateAndEdit") {
+ verifyReviewFormula(t, a, credential, seed.ID, true, false)
+ } else if strings.Contains(only, "FailedCellEdit") {
+ verifyReviewFormula(t, a, credential, seed.ID, false, true)
+ if !refusedSyntheticEdit.Load() {
+ t.Fatal("Retry UI did not exercise the isolated write refusal")
+ }
+ } else if strings.Contains(only, "Export") && !requestedExport.Load() {
+ t.Fatal("Native export control did not reach the isolated server")
}
current := strings.TrimSpace(string(run("get_app_container", simulator, bundle, "data")))
- staged, err := filepath.Glob(filepath.Join(current, "tmp", "oficina-sheets-export-*.xlsx"))
+ staged, err := filepath.Glob(filepath.Join(current, "tmp", "oficina-sheets-export-*"))
if err != nil || len(staged) != 0 {
t.Fatal("Cancelled native export left an unsealed staged workbook")
}
@@ -164,7 +185,7 @@ func TestNativeReview(t *testing.T) {
}
for _, test := range manifest {
for _, attachment := range test.Attachments {
- if strings.Contains(attachment.SuggestedHumanReadableName, "Sheets native formula editing") {
+ if strings.Contains(attachment.SuggestedHumanReadableName, "Sheets native") {
image, err := os.ReadFile(filepath.Join(attachments, filepath.Base(attachment.ExportedFileName)))
if err != nil {
t.Fatal(err)
@@ -222,7 +243,7 @@ func reviewNativeCredential(t *testing.T, handler http.Handler, browser []*http.
// Verify the UI's result through a fresh authenticated read, so rendering a
// optimistic value without persistence cannot satisfy this fixture.
-func verifyReviewFormula(t *testing.T, a *App, credential map[string]string, seedID uint64) {
+func verifyReviewFormula(t *testing.T, a *App, credential map[string]string, seedID uint64, requireMain, requireRetry bool) {
t.Helper()
get := func(path string) *httptest.ResponseRecorder {
request := nativeGet(path)
@@ -261,7 +282,7 @@ func verifyReviewFormula(t *testing.T, a *App, credential map[string]string, see
verifiedMain = true
}
}
- if verifiedMain && verifiedRetry {
+ if (!requireMain || verifiedMain) && (!requireRetry || verifiedRetry) {
return
}
t.Fatal("Native formula, fill, range paste, formatting and history restore did not persist")
diff --git a/native/REVIEW.md b/native/REVIEW.md
index 7990066..2673991 100644
--- a/native/REVIEW.md
+++ b/native/REVIEW.md
@@ -23,12 +23,22 @@ Set `NATIVE_REVIEW_SIMULATOR` to a booted iPhone or iPad simulator UUID, then ru
xcrun simctl install "$NATIVE_REVIEW_SIMULATOR" \
native/build/review-ios/Build/Products/Debug-iphonesimulator/SheetsReviewPhone.app
NATIVE_REVIEW_UI_TESTS=1 NATIVE_REVIEW_OUTPUT=/tmp/sheets-native-review.png \
- go test ./internal/sheets -run '^TestNativeReview$' -count=1 -timeout 5m
+ go test ./internal/sheets -run '^TestNativeReview$' -count=1 -timeout 8m
```
-The UI test creates a sheet, enters `=2+3`, checks `5`, selects the cell to
-read its formula, changes it to `=7*6`, and checks `42`. A separate authenticated
-server read verifies that the final raw formula and computed value persisted.
+The UI cases create a sheet, enter `=2+3`, check `5`, select the cell to read its
+formula, change it to `=7*6`, and check `42`. They then exercise fill down,
+range copy/paste, number formatting, own input undo and version-history restore.
+A separate case injects one HTTP 503 for the exact synthetic formula `=6*7`,
+checks that its draft remains visible and protected, and retries it. Export
+opens the real system destination picker and cancels; the fixture verifies
+the export request reached the server and no staged XLSX remains afterward.
+It does not test saving a file to an external destination.
+A fresh authenticated server read verifies the resulting raw formulas and
+computed values. To isolate a case, set `NATIVE_REVIEW_UI_ONLY` to
+`NativeUITests/testCreateAndEditFormula`,
+`NativeUITests/testFailedCellEditCanBeRetriedWithoutLosingDraft`, or
+`NativeUITests/testExportWorkbookOpensNativeDestinationAndCancels`.
The fixture exports the test's screenshot attachment to `NATIVE_REVIEW_OUTPUT`.
Its xcodebuild log is stored under the operating system temporary directory
as `sheets-native-ui-<simulator UUID>.log`.
@@ -47,7 +57,19 @@ checks access and does not request it automatically. On 2026-09-13, the review
executable compiled and authenticated, but this environment lacked capture
access; no Mac screenshot was accepted as verification.
-On 2026-09-13 the UI flow and server persistence check passed on iPhone 17 Pro
+On 2026-09-13 the initial formula UI flow and server persistence check passed on iPhone 17 Pro
and iPad Pro 13-inch (M5), iOS 26.5. The same test first exposed compact-screen
navigation and cell-selection failures before their fixes. This exercises the
named flow, not every feature in the parity inventory.
+The expanded three-case iPad UI suite passed after exposing a blank history
+panel and an export filename crash. Its combined Xcode result finalization
+then timed out; separate fixture runs distinguish UI assertions from complete
+server-persistence and cleanup verification. The isolated export case passed
+including its server and temporary-file checks.
+The isolated expanded editing case subsequently passed in 192.565 seconds,
+including its fresh 2×2 formula/format/history checks; the isolated refused-write
+retry passed in 45.066 seconds, including verification that `=6*7` persisted.
+The final export stores the workbook under its title inside a private unique
+directory. Its isolated test passed in 58.071 seconds after a simulator restart,
+including removal of that directory after cancellation. The screenshot was
+visually checked: the system picker displays “Native review workbook”.
diff --git a/native/apple/Sources/SheetsKit/SheetFiles.swift b/native/apple/Sources/SheetsKit/SheetFiles.swift
index 7196fbb..da8ef7b 100644
--- a/native/apple/Sources/SheetsKit/SheetFiles.swift
+++ b/native/apple/Sources/SheetsKit/SheetFiles.swift
@@ -7,12 +7,15 @@ struct WorkbookExport: Transferable {
let filename: String
init(url: URL, title: String) {
self.url = url
+ filename = Self.filename(for: title)
+ }
+ static func filename(for title: String) -> String {
var name = title.components(separatedBy: .controlCharacters).joined()
.replacingOccurrences(of: "/", with: "-").replacingOccurrences(of: ":", with: "-")
.trimmingCharacters(in: .whitespacesAndNewlines.union(CharacterSet(charactersIn: ".")))
while name.utf8.count > 200 { name.removeLast() }
if name.isEmpty { name = "Workbook" }
- filename = name.lowercased().hasSuffix(".xlsx") ? name : name + ".xlsx"
+ return name.lowercased().hasSuffix(".xlsx") ? name : name + ".xlsx"
}
static var transferRepresentation: some TransferRepresentation {
FileRepresentation(exportedContentType: UTType(filenameExtension: "xlsx") ?? .data) {
@@ -75,7 +78,7 @@ struct SheetFiles: View {
}
}
private func cleanupExport() {
- if let workbook { try? FileManager.default.removeItem(at: workbook.url) }
+ if let workbook { try? FileManager.default.removeItem(at: workbook.url.deletingLastPathComponent()) }
workbook = nil
}
private func export() async {
@@ -85,18 +88,22 @@ struct SheetFiles: View {
}
working = true
defer { working = false }
- let destination = FileManager.default.temporaryDirectory
- .appendingPathComponent("oficina-sheets-export-" + UUID().uuidString + ".xlsx")
+ let directory = FileManager.default.temporaryDirectory
+ .appendingPathComponent("oficina-sheets-export-" + UUID().uuidString, isDirectory: true)
+ let title = model.page?.Sheet.Title ?? "Workbook"
+ let destination = directory.appendingPathComponent(WorkbookExport.filename(for: title))
do {
+ try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: false,
+ attributes: [.posixPermissions: 0o700])
try await model.client.download(model.base + "/export.xlsx", to: destination)
guard active else {
- try? FileManager.default.removeItem(at: destination)
+ try? FileManager.default.removeItem(at: directory)
return
}
- workbook = WorkbookExport(url: destination, title: model.page?.Sheet.Title ?? "Workbook")
+ workbook = WorkbookExport(url: destination, title: title)
exporter = true
} catch {
- try? FileManager.default.removeItem(at: destination)
+ try? FileManager.default.removeItem(at: directory)
progress = error.localizedDescription
}
}
diff --git a/native/ios/UITests/NativeUITests.swift b/native/ios/UITests/NativeUITests.swift
index 02ef41b..6235cd3 100644
--- a/native/ios/UITests/NativeUITests.swift
+++ b/native/ios/UITests/NativeUITests.swift
@@ -5,6 +5,7 @@ import XCTest
continueAfterFailure = false
let app = XCUIApplication()
app.launch()
+ defer { app.terminate() }
let create = app.buttons["New sheet"]
XCTAssertTrue(create.waitForExistence(timeout: 15))
create.tap()
@@ -76,6 +77,7 @@ import XCTest
continueAfterFailure = false
let app = XCUIApplication()
app.launch()
+ defer { app.terminate() }
XCTAssertTrue(app.buttons["New sheet"].waitForExistence(timeout: 15))
app.buttons["New sheet"].tap()
let formula = app.textFields["Cell contents or formula"]
@@ -93,12 +95,15 @@ import XCTest
retry.tap()
XCTAssertTrue(app.buttons["A1, 42"].waitForExistence(timeout: 10))
XCTAssertEqual(XCTWaiter.wait(for: [XCTNSPredicateExpectation(predicate: NSPredicate(format: "exists == false"), object: retry)], timeout: 10), .completed)
+ let screenshot = XCTAttachment(screenshot: app.screenshot())
+ screenshot.name = "Sheets native retried edit"; screenshot.lifetime = .keepAlways; add(screenshot)
}
func testExportWorkbookOpensNativeDestinationAndCancels() {
continueAfterFailure = false
let app = XCUIApplication()
app.launch()
+ defer { app.terminate() }
let workbook = app.buttons["Native review workbook"]
XCTAssertTrue(workbook.waitForExistence(timeout: 15))
workbook.tap()