Skip to content

Commit 13f1da5

Browse files
committed
fix(testflight): preserve post-upload notes recovery
1 parent 85af115 commit 13f1da5

8 files changed

Lines changed: 355 additions & 2 deletions

File tree

internal/asc/output_publish.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@ import (
55
"strings"
66
)
77

8+
// TestNotesRecovery describes a shell-neutral retry after a build exists but
9+
// setting its What to Test notes fails.
10+
type TestNotesRecovery struct {
11+
BuildID string `json:"buildId"`
12+
Locale string `json:"locale"`
13+
SubmittedNotes string `json:"submittedNotes"`
14+
Command string `json:"command"`
15+
Arguments []string `json:"arguments"`
16+
}
17+
818
func testFlightPublishResultRows(result *TestFlightPublishResult) ([]string, [][]string) {
919
headers := []string{"Build ID", "Version", "Build Number", "Processing", "Groups", "Uploaded", "Notified", "Notification Action", "Beta Review Submitted", "Beta Review Submission ID"}
1020
notified := ""

internal/asc/publish.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ type TestFlightPublishResult struct {
7474
FailureStage string `json:"failureStage,omitempty"`
7575
Failure string `json:"failure,omitempty"`
7676
CompletedStages []string `json:"completedStages,omitempty"`
77+
Recovery *TestNotesRecovery `json:"recovery,omitempty"`
7778
Mode PublishMode `json:"mode,omitempty"`
7879
BuildID string `json:"buildId"`
7980
BuildVersion string `json:"buildVersion,omitempty"`

internal/cli/builds/builds_commands.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ Examples:
365365
if testNotesValue != "" {
366366
fmt.Fprintf(os.Stderr, "Build %s discovered; setting What to Test notes...\n", buildResp.Data.ID)
367367
if _, err := shared.UpsertBetaBuildLocalization(requestCtx, client, buildResp.Data.ID, localeValue, testNotesValue); err != nil {
368-
return fmt.Errorf("builds upload: %w", err)
368+
return fmt.Errorf("builds upload: %w", shared.NewTestNotesRecoveryError(buildResp.Data.ID, localeValue, testNotesValue, err))
369369
}
370370
}
371371

internal/cli/cmdtest/builds_upload_wait_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,88 @@ func TestBuildsUploadTestNotesWritesBeforeProcessingCompletes(t *testing.T) {
330330
}
331331
}
332332

333+
func TestBuildsUploadTestNotesFailurePreservesDiscoveredBuildRecoveryContext(t *testing.T) {
334+
setupAuth(t)
335+
t.Setenv("ASC_CONFIG_PATH", filepath.Join(t.TempDir(), "nonexistent.json"))
336+
testNotes := "Line one\nLine two\x1b[31m"
337+
338+
ipaPath := writeBuildUploadIPA(t, "com.example.demo")
339+
340+
originalTransport := http.DefaultTransport
341+
t.Cleanup(func() {
342+
http.DefaultTransport = originalTransport
343+
})
344+
345+
http.DefaultTransport = roundTripFunc(func(req *http.Request) (*http.Response, error) {
346+
switch {
347+
case req.Method == http.MethodGet && req.URL.Path == "/v1/apps/123456789":
348+
return jsonResponse(http.StatusOK, `{"data":{"type":"apps","id":"123456789","attributes":{"name":"Demo","bundleId":"com.example.demo"}}}`)
349+
case req.Method == http.MethodPost && req.URL.Path == "/v1/buildUploads":
350+
return jsonResponse(http.StatusOK, `{"data":{"type":"buildUploads","id":"upload-1","attributes":{"cfBundleShortVersionString":"1.0.0","cfBundleVersion":"42","platform":"IOS"}}}`)
351+
case req.Method == http.MethodPost && req.URL.Path == "/v1/buildUploadFiles":
352+
return jsonResponse(http.StatusOK, `{"data":{"type":"buildUploadFiles","id":"file-1","attributes":{"fileName":"app.ipa","fileSize":4,"uti":"com.apple.itunes.ipa","assetType":"ASSET","uploadOperations":[{"method":"PUT","url":"https://upload.example.com/part-1","length":4,"offset":0,"requestHeaders":[{"name":"Content-Type","value":"application/octet-stream"}]}]}}}`)
353+
case req.Method == http.MethodPut && req.URL.Host == "upload.example.com":
354+
return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader("")), Header: http.Header{}}, nil
355+
case req.Method == http.MethodPatch && req.URL.Path == "/v1/buildUploadFiles/file-1":
356+
return jsonResponse(http.StatusOK, `{"data":{"type":"buildUploadFiles","id":"file-1","attributes":{"uploaded":true}}}`)
357+
case req.Method == http.MethodGet && req.URL.Path == "/v1/buildUploads/upload-1":
358+
return jsonResponse(http.StatusOK, `{"data":{"type":"buildUploads","id":"upload-1","relationships":{"build":{"data":{"type":"builds","id":"build-1"}}}}}`)
359+
case req.Method == http.MethodGet && req.URL.Path == "/v1/builds/build-1":
360+
return jsonResponse(http.StatusOK, `{"data":{"type":"builds","id":"build-1","attributes":{"version":"42","processingState":"PROCESSING"}}}`)
361+
case req.Method == http.MethodGet && req.URL.Path == "/v1/builds/build-1/betaBuildLocalizations":
362+
return jsonResponse(http.StatusOK, `{"data":[]}`)
363+
case req.Method == http.MethodPost && req.URL.Path == "/v1/betaBuildLocalizations":
364+
return jsonResponse(http.StatusUnprocessableEntity, `{"errors":[{"status":"422","code":"ENTITY_ERROR.ATTRIBUTE.INVALID","title":"The provided entity has an invalid attribute","detail":"What to Test was rejected by the server"}]}`)
365+
default:
366+
t.Fatalf("unexpected request: %s %s", req.Method, req.URL.String())
367+
return nil, nil
368+
}
369+
})
370+
371+
root := RootCommand("1.2.3")
372+
root.FlagSet.SetOutput(io.Discard)
373+
374+
var runErr error
375+
stdout, _ := captureOutput(t, func() {
376+
if err := root.Parse([]string{
377+
"builds", "upload",
378+
"--app", "123456789",
379+
"--ipa", ipaPath,
380+
"--version", "1.0.0",
381+
"--build-number", "42",
382+
"--test-notes", testNotes,
383+
"--locale", "en-US",
384+
"--poll-interval", "1ms",
385+
}); err != nil {
386+
t.Fatalf("parse error: %v", err)
387+
}
388+
runErr = root.Run(context.Background())
389+
})
390+
391+
if runErr == nil {
392+
t.Fatal("expected test notes rejection after upload")
393+
}
394+
if stdout != "" {
395+
t.Fatalf("expected no new structured output contract on failure, got %q", stdout)
396+
}
397+
wantParts := []string{
398+
`build "build-1" is available`,
399+
`locale "en-US"`,
400+
"The provided entity has an invalid attribute: What to Test was rejected by the server",
401+
"retry without uploading the build again",
402+
"reuse the original notes",
403+
"asc builds test-notes create --build-id BUILD_ID --locale LOCALE --whats-new NOTES",
404+
}
405+
for _, want := range wantParts {
406+
if !strings.Contains(runErr.Error(), want) {
407+
t.Fatalf("expected recovery error to contain %q, got %v", want, runErr)
408+
}
409+
}
410+
if asc.HasInterpretedTerminalSequence(runErr.Error()) || strings.Contains(runErr.Error(), "Line one") {
411+
t.Fatalf("expected sanitized human recovery without embedded notes, got %q", runErr)
412+
}
413+
}
414+
333415
func TestBuildsUploadPostCommitVerificationUsesFreshTimeoutWindow(t *testing.T) {
334416
setupAuth(t)
335417
t.Setenv("ASC_CONFIG_PATH", filepath.Join(t.TempDir(), "nonexistent.json"))

internal/cli/publish/publish.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,9 @@ Examples:
403403

404404
if testNotesValue != "" {
405405
if _, err := shared.UpsertBetaBuildLocalization(requestCtx, client, buildResp.Data.ID, localeValue, testNotesValue); err != nil {
406-
return reportPartialFailure(publishFailureStageTestNotes, fmt.Errorf("publish testflight: %w", err))
406+
recoveryErr := shared.NewTestNotesRecoveryError(buildResp.Data.ID, localeValue, testNotesValue, err)
407+
result.Recovery = recoveryErr.Recovery()
408+
return reportPartialFailure(publishFailureStageTestNotes, fmt.Errorf("publish testflight: %w", recoveryErr))
407409
}
408410
completedStages = append(completedStages, publishCompletedStageTestNotes)
409411
}

internal/cli/publish/publish_local_build_test.go

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -774,6 +774,149 @@ func TestPublishTestFlightUploadReportsStructuredRecoveryResultAfterBetaReviewFa
774774
}
775775
}
776776

777+
func TestPublishTestFlightUploadTestNotesFailurePreservesStructuredRecoveryContext(t *testing.T) {
778+
restore := overridePublishCommandTestHooks(t)
779+
defer restore()
780+
testNotes := "Line one\nLine two\x1b[31m"
781+
782+
getPublishASCClientFn = func(time.Duration) (*asc.Client, error) { return newPublishCommandTestClient(t), nil }
783+
resolvePublishAppIDWithLookupFn = func(_ context.Context, _ *asc.Client, _ string) (string, error) {
784+
return "app-123", nil
785+
}
786+
validatePublishIPAPathFn = func(string) (os.FileInfo, error) {
787+
return newPublishTestFileInfo(t)
788+
}
789+
uploadCalls := 0
790+
uploadBuildAndWaitForIDFn = func(_ context.Context, _ *asc.Client, _ string, _ string, _ os.FileInfo, version, buildNumber string, _ asc.Platform, _ time.Duration, _ time.Duration, _ bool) (*publishUploadResult, error) {
791+
uploadCalls++
792+
return &publishUploadResult{
793+
Build: &asc.BuildResponse{Data: asc.Resource[asc.BuildAttributes]{
794+
ID: "build-123",
795+
Attributes: asc.BuildAttributes{
796+
Version: buildNumber,
797+
ProcessingState: asc.BuildProcessingStateProcessing,
798+
},
799+
}},
800+
Version: version,
801+
BuildNumber: buildNumber,
802+
}, nil
803+
}
804+
waitForPublishBuildProcessingFn = func(context.Context, *asc.Client, string, time.Duration) (*asc.BuildResponse, error) {
805+
return &asc.BuildResponse{Data: asc.Resource[asc.BuildAttributes]{
806+
ID: "build-123",
807+
Attributes: asc.BuildAttributes{
808+
Version: "42",
809+
ProcessingState: asc.BuildProcessingStateValid,
810+
},
811+
}}, nil
812+
}
813+
814+
originalTransport := http.DefaultTransport
815+
t.Cleanup(func() { http.DefaultTransport = originalTransport })
816+
requestCount := 0
817+
http.DefaultTransport = publishCommandRoundTripFunc(func(req *http.Request) (*http.Response, error) {
818+
requestCount++
819+
switch requestCount {
820+
case 1:
821+
if req.Method != http.MethodGet || req.URL.Path != "/v1/apps/app-123/betaGroups" {
822+
t.Fatalf("unexpected request %d: %s %s", requestCount, req.Method, req.URL.String())
823+
}
824+
return publishCommandJSONResponse(http.StatusOK, `{"data":[{"type":"betaGroups","id":"group-1","attributes":{"name":"External","isInternalGroup":false}}]}`)
825+
case 2:
826+
if req.Method != http.MethodGet || req.URL.Path != "/v1/builds/build-123/betaBuildLocalizations" {
827+
t.Fatalf("unexpected request %d: %s %s", requestCount, req.Method, req.URL.String())
828+
}
829+
return publishCommandJSONResponse(http.StatusOK, `{"data":[]}`)
830+
case 3:
831+
if req.Method != http.MethodPost || req.URL.Path != "/v1/betaBuildLocalizations" {
832+
t.Fatalf("unexpected request %d: %s %s", requestCount, req.Method, req.URL.String())
833+
}
834+
return publishCommandJSONResponse(http.StatusUnprocessableEntity, `{"errors":[{"status":"422","code":"ENTITY_ERROR.ATTRIBUTE.INVALID","title":"The provided entity has an invalid attribute","detail":"What to Test was rejected by the server"}]}`)
835+
default:
836+
t.Fatalf("unexpected request %d: %s %s", requestCount, req.Method, req.URL.String())
837+
return nil, nil
838+
}
839+
})
840+
841+
cmd := PublishTestFlightCommand()
842+
cmd.FlagSet.SetOutput(io.Discard)
843+
if err := cmd.FlagSet.Parse([]string{
844+
"--app", "app-123",
845+
"--ipa", "Demo.ipa",
846+
"--version", "1.2.3",
847+
"--build-number", "42",
848+
"--group", "External",
849+
"--test-notes", testNotes,
850+
"--locale", "en-US",
851+
"--output", "json",
852+
}); err != nil {
853+
t.Fatalf("parse flags: %v", err)
854+
}
855+
856+
var runErr error
857+
stdout, _ := capturePublishCommandOutput(t, func() error {
858+
runErr = cmd.Exec(context.Background(), nil)
859+
return runErr
860+
})
861+
if runErr == nil {
862+
t.Fatal("expected post-upload test notes rejection")
863+
}
864+
if uploadCalls != 1 {
865+
t.Fatalf("upload calls = %d, want 1", uploadCalls)
866+
}
867+
wantParts := []string{
868+
`build "build-123" is available`,
869+
`locale "en-US"`,
870+
"The provided entity has an invalid attribute: What to Test was rejected by the server",
871+
"retry without uploading the build again",
872+
"reuse the original notes",
873+
"asc builds test-notes create --build-id BUILD_ID --locale LOCALE --whats-new NOTES",
874+
}
875+
for _, want := range wantParts {
876+
if !strings.Contains(runErr.Error(), want) {
877+
t.Fatalf("expected recovery error to contain %q, got %v", want, runErr)
878+
}
879+
}
880+
if asc.HasInterpretedTerminalSequence(runErr.Error()) || strings.Contains(runErr.Error(), "Line one") {
881+
t.Fatalf("expected sanitized human recovery without embedded notes, got %q", runErr)
882+
}
883+
884+
var result asc.TestFlightPublishResult
885+
if err := json.Unmarshal([]byte(stdout), &result); err != nil {
886+
t.Fatalf("decode partial publish result: %v\nstdout=%s", err, stdout)
887+
}
888+
if result.Status != publishPartialStatus || result.FailureStage != publishFailureStageTestNotes {
889+
t.Fatalf("unexpected partial status: status=%q stage=%q", result.Status, result.FailureStage)
890+
}
891+
if result.BuildID != "build-123" || !result.Uploaded {
892+
t.Fatalf("expected recoverable uploaded build, got buildId=%q uploaded=%t", result.BuildID, result.Uploaded)
893+
}
894+
wantCompleted := []string{publishCompletedStageUpload, publishCompletedStageBuildProcessing}
895+
if !slices.Equal(result.CompletedStages, wantCompleted) {
896+
t.Fatalf("completed stages = %v, want %v", result.CompletedStages, wantCompleted)
897+
}
898+
for _, want := range wantParts {
899+
if !strings.Contains(result.Failure, want) {
900+
t.Fatalf("expected structured failure to contain %q, got %q", want, result.Failure)
901+
}
902+
}
903+
if result.Recovery == nil {
904+
t.Fatalf("expected typed test-notes recovery, got %#v", result)
905+
}
906+
if result.Recovery.BuildID != "build-123" || result.Recovery.Locale != "en-US" || result.Recovery.SubmittedNotes != testNotes {
907+
t.Fatalf("structured recovery lost exact values: %#v", result.Recovery)
908+
}
909+
wantArguments := []string{
910+
"builds", "test-notes", "create",
911+
"--build-id", "build-123",
912+
"--locale", "en-US",
913+
"--whats-new", testNotes,
914+
}
915+
if result.Recovery.Command != "asc" || !slices.Equal(result.Recovery.Arguments, wantArguments) {
916+
t.Fatalf("structured recovery command = %q %#v, want asc %#v", result.Recovery.Command, result.Recovery.Arguments, wantArguments)
917+
}
918+
}
919+
777920
func TestPublishTestFlightUploadReportsTerminalProcessingStateAfterWaitFailure(t *testing.T) {
778921
restore := overridePublishCommandTestHooks(t)
779922
defer restore()

internal/cli/shared/test_notes.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,59 @@ func UpsertBetaBuildLocalization(ctx context.Context, client *asc.Client, buildI
5252
}
5353
return client.CreateBetaBuildLocalization(ctx, buildID, attrs)
5454
}
55+
56+
// TestNotesRecoveryError preserves a discovered build and the exact retry
57+
// arguments while keeping its human-facing diagnostic terminal-safe.
58+
type TestNotesRecoveryError struct {
59+
buildID string
60+
locale string
61+
notes string
62+
cause error
63+
}
64+
65+
// NewTestNotesRecoveryError returns recovery context for a failed post-upload
66+
// What to Test request.
67+
func NewTestNotesRecoveryError(buildID, locale, notes string, cause error) *TestNotesRecoveryError {
68+
return &TestNotesRecoveryError{
69+
buildID: buildID,
70+
locale: locale,
71+
notes: notes,
72+
cause: cause,
73+
}
74+
}
75+
76+
func (e *TestNotesRecoveryError) Error() string {
77+
buildID := asc.SanitizeTerminalText(e.buildID)
78+
locale := asc.SanitizeTerminalText(e.locale)
79+
cause := "unknown error"
80+
if e.cause != nil {
81+
cause = asc.SanitizeTerminalText(e.cause.Error())
82+
}
83+
return fmt.Sprintf(
84+
"build %q is available, but setting What to Test notes for locale %q failed: %s; retry without uploading the build again and reuse the original notes: asc builds test-notes create --build-id BUILD_ID --locale LOCALE --whats-new NOTES",
85+
buildID,
86+
locale,
87+
cause,
88+
)
89+
}
90+
91+
// Unwrap preserves API error status and exit classification.
92+
func (e *TestNotesRecoveryError) Unwrap() error {
93+
return e.cause
94+
}
95+
96+
// Recovery returns exact, shell-neutral retry data for structured output.
97+
func (e *TestNotesRecoveryError) Recovery() *asc.TestNotesRecovery {
98+
return &asc.TestNotesRecovery{
99+
BuildID: e.buildID,
100+
Locale: e.locale,
101+
SubmittedNotes: e.notes,
102+
Command: "asc",
103+
Arguments: []string{
104+
"builds", "test-notes", "create",
105+
"--build-id", e.buildID,
106+
"--locale", e.locale,
107+
"--whats-new", e.notes,
108+
},
109+
}
110+
}

0 commit comments

Comments
 (0)