Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 107 additions & 0 deletions internal/cli/pricing/diagnostics_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package pricing

import (
"context"
"errors"
"flag"
"testing"

"github.com/rudrankriyam/App-Store-Connect-CLI/internal/cli/shared"
)

func TestPricingScheduleCreateInvalidInputExposesStructuredDiagnostics(t *testing.T) {
t.Setenv("ASC_APP_ID", "")

tests := []struct {
name string
args []string
wantError string
wantStderr string
wantUsage bool
wantCode shared.DiagnosticCode
wantParam string
}{
{
name: "no price selection",
args: []string{"--app", "APP", "--base-territory", "USA", "--start-date", "2024-03-01"},
wantStderr: "Error: one of --price-point, --tier, --price, or --free is required\n",
wantUsage: true,
wantCode: shared.DiagnosticRequiredInputMissing,
wantParam: "",
},
{
name: "conflicting price selection",
args: []string{"--app", "APP", "--price-point", "PP", "--price", "0.99", "--base-territory", "USA", "--start-date", "2024-03-01"},
wantStderr: "Error: --price-point, --tier, --price, and --free are mutually exclusive\n",
wantUsage: true,
wantCode: shared.DiagnosticConflictingInput,
wantParam: "",
},
{
name: "negative tier",
args: []string{"--app", "APP", "--tier", "-1", "--base-territory", "USA", "--start-date", "2024-03-01"},
wantStderr: "Error: --tier must be a positive integer\n",
wantUsage: true,
wantCode: shared.DiagnosticInvalidInput,
wantParam: "--tier",
},
{
name: "non numeric price",
args: []string{"--app", "APP", "--price", "abc", "--base-territory", "USA", "--start-date", "2024-03-01"},
wantStderr: "Error: --price must be a number\n",
wantUsage: true,
wantCode: shared.DiagnosticInvalidInput,
wantParam: "--price",
},
{
name: "unmappable base territory",
args: []string{"--app", "APP", "--price-point", "PP", "--base-territory", "Neverland", "--start-date", "2024-03-01"},
wantError: "territory \"Neverland\" could not be mapped to an App Store Connect territory ID",
wantStderr: "Error: territory \"Neverland\" could not be mapped to an App Store Connect territory ID\n",
wantUsage: true,
wantCode: shared.DiagnosticInvalidInput,
wantParam: "--base-territory",
},
{
name: "malformed start date",
args: []string{"--app", "APP", "--price-point", "PP", "--base-territory", "USA", "--start-date", "03-01-2024"},
wantError: "pricing schedule create: --start-date must be in YYYY-MM-DD format",
wantCode: shared.DiagnosticInvalidInput,
wantParam: "--start-date",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
command := PricingScheduleCreateCommand()
if err := command.FlagSet.Parse(test.args); err != nil {
t.Fatalf("parse flags: %v", err)
}

var err error
stderr := capturePricingStderr(t, func() {
err = command.Exec(context.Background(), nil)
})
if err == nil {
t.Fatal("expected error")
}
if test.wantError != "" && err.Error() != test.wantError {
t.Fatalf("error = %q, want %q", err, test.wantError)
}
if test.wantStderr != "" && stderr != test.wantStderr {
t.Fatalf("stderr = %q, want %q", stderr, test.wantStderr)
}
if got := errors.Is(err, flag.ErrHelp); got != test.wantUsage {
t.Fatalf("errors.Is(err, flag.ErrHelp) = %t, want %t", got, test.wantUsage)
}

diagnostic, ok := shared.DiagnosticFromError(err)
if !ok {
t.Fatalf("DiagnosticFromError(%v) found no metadata", err)
}
if diagnostic.Code != test.wantCode || diagnostic.Parameter != test.wantParam {
t.Fatalf("diagnostic = %+v, want code %q parameter %q", diagnostic, test.wantCode, test.wantParam)
}
})
}
}
86 changes: 86 additions & 0 deletions internal/cli/reviews/diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,92 @@ func TestReviewValidationDiagnosticsPreserveErrorContracts(t *testing.T) {
}
}

func TestReviewSubmitInputFailuresKeepStructuredDiagnostics(t *testing.T) {
t.Setenv("ASC_APP_ID", "")

tests := []struct {
name string
args []string
wantStderr string
wantCode shared.DiagnosticCode
wantParam string
}{
{
name: "missing app",
args: nil,
wantStderr: "Error: --app is required (or set ASC_APP_ID)\n",
wantCode: shared.DiagnosticRequiredInputMissing,
wantParam: "--app",
},
{
name: "missing build",
args: []string{"--app", "123456789"},
wantStderr: "Error: --build is required\n",
wantCode: shared.DiagnosticRequiredInputMissing,
wantParam: "--build",
},
{
name: "missing version selector",
args: []string{"--app", "123456789", "--build", "BUILD_ID"},
wantStderr: "Error: --version or --version-id is required\n",
wantCode: shared.DiagnosticRequiredInputMissing,
wantParam: "",
},
{
name: "conflicting version selectors",
args: []string{"--app", "123456789", "--build", "BUILD_ID", "--version", "1.2.3", "--version-id", "VERSION_ID"},
wantStderr: "Error: --version and --version-id are mutually exclusive\n",
wantCode: shared.DiagnosticConflictingInput,
wantParam: "",
},
{
name: "missing confirm",
args: []string{"--app", "123456789", "--build", "BUILD_ID", "--version", "1.2.3"},
wantStderr: "Error: --confirm is required unless --dry-run is set\n",
wantCode: shared.DiagnosticRequiredInputMissing,
wantParam: "--confirm",
},
{
name: "unsupported platform",
args: []string{"--app", "123456789", "--build", "BUILD_ID", "--version", "1.2.3", "--dry-run", "--platform", "WATCH_OS"},
wantStderr: "Error: --platform must be one of: IOS, MAC_OS, TV_OS, VISION_OS\n",
wantCode: shared.DiagnosticInvalidInput,
wantParam: "--platform",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
command := ReviewSubmitCommand()
if err := command.FlagSet.Parse(test.args); err != nil {
t.Fatalf("parse flags: %v", err)
}

var err error
stderr := captureReviewDiagnosticStderr(t, func() {
err = command.Exec(context.Background(), nil)
})
if err == nil {
t.Fatal("expected error")
}
if stderr != test.wantStderr {
t.Fatalf("stderr = %q, want %q", stderr, test.wantStderr)
}
if !errors.Is(err, flag.ErrHelp) {
t.Fatalf("error = %v, want flag.ErrHelp usage contract", err)
}

diagnostic, ok := shared.DiagnosticFromError(err)
if !ok {
t.Fatalf("DiagnosticFromError(%v) found no metadata", err)
}
if diagnostic.Code != test.wantCode || diagnostic.Parameter != test.wantParam {
t.Fatalf("diagnostic = %+v, want code %q parameter %q", diagnostic, test.wantCode, test.wantParam)
}
})
}
}

func TestReviewAttachmentUnreadableSourceIsDiagnosedBeforeAuth(t *testing.T) {
attachmentPath := filepath.Join(t.TempDir(), "attachment.pdf")
if err := os.WriteFile(attachmentPath, []byte("review attachment"), 0o600); err != nil {
Expand Down
12 changes: 12 additions & 0 deletions internal/cli/shared/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,18 @@ func InvalidValueUsageError(parameters ...string) error {
)
}

// reportedUsageErrHelp preserves the flag.ErrHelp usage contract for a
// validation failure whose message has already been written to stderr, while
// forwarding any structured diagnostic the validator attached so telemetry
// keeps the failing parameter.
func reportedUsageErrHelp(err error) error {
diagnostic, ok := DiagnosticFromError(err)
if !ok {
return flag.ErrHelp
}
return WithDiagnostic(flag.ErrHelp, diagnostic.Code, diagnostic.Parameter)
}

func ClassifyUsageError(err error) UsageErrorKind {
var classified interface{ UsageErrorKind() UsageErrorKind }
if errors.As(err, &classified) {
Expand Down
12 changes: 8 additions & 4 deletions internal/cli/shared/pricing_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ func NewPricingSetCommand(config PricingSetCommandConfig) *ffcli.Command {

if err := ValidatePriceSelectionFlags(pricePointValue, tierValue, priceValue, freeValue); err != nil {
fmt.Fprintln(os.Stderr, "Error:", err)
return flag.ErrHelp
return reportedUsageErrHelp(err)
}
if err := ValidateFinitePriceFlag("--price", priceValue); err != nil {
fmt.Fprintln(os.Stderr, "Error:", err)
return flag.ErrHelp
return WithDiagnostic(flag.ErrHelp, DiagnosticInvalidInput, "--price")
}

baseTerritoryInput := strings.TrimSpace(*baseTerritory)
Expand All @@ -78,7 +78,7 @@ func NewPricingSetCommand(config PricingSetCommandConfig) *ffcli.Command {
if baseTerritoryInput != "" {
normalizedBaseTerritory, normalizeErr := ascterritory.Normalize(baseTerritoryInput)
if normalizeErr != nil {
return UsageError(normalizeErr.Error())
return WithDiagnostic(UsageError(normalizeErr.Error()), DiagnosticInvalidInput, "--base-territory")
}
baseTerritoryValue = normalizedBaseTerritory
}
Expand All @@ -95,7 +95,11 @@ func NewPricingSetCommand(config PricingSetCommandConfig) *ffcli.Command {

normalizedStartDate, err := normalizePricingStartDate(startDateValue)
if err != nil {
return fmt.Errorf("%s: %w", config.ErrorPrefix, err)
return WithDiagnostic(
fmt.Errorf("%s: %w", config.ErrorPrefix, err),
DiagnosticInvalidInput,
"--start-date",
)
}

client, err := getASCClient()
Expand Down
10 changes: 6 additions & 4 deletions internal/cli/shared/tier_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,10 +358,12 @@ func ResolvePricePointByPrice(tiers []TierEntry, price string) (string, error) {
}

// ValidatePriceSelectionFlags checks that --price-point, --tier, --price, and --free are mutually exclusive.
// Returns a usage-style error if more than one is set.
// Returns a usage-style error if more than one is set. Each failure carries a
// structured diagnostic so callers can classify it without re-deriving which
// rule was violated; the rendered messages are unchanged.
func ValidatePriceSelectionFlags(pricePoint string, tier int, price string, free ...bool) error {
if tier < 0 {
return fmt.Errorf("--tier must be a positive integer")
return WithDiagnostic(fmt.Errorf("--tier must be a positive integer"), DiagnosticInvalidInput, "--tier")
}

supportsFree := len(free) > 0
Expand All @@ -382,10 +384,10 @@ func ValidatePriceSelectionFlags(pricePoint string, tier int, price string, free
count++
}
if count == 0 {
return fmt.Errorf("%s", requiredMessage)
return WithDiagnostic(fmt.Errorf("%s", requiredMessage), DiagnosticRequiredInputMissing, "")
}
if count > 1 {
return fmt.Errorf("%s", mutuallyExclusiveMessage)
return WithDiagnostic(fmt.Errorf("%s", mutuallyExclusiveMessage), DiagnosticConflictingInput, "")
}
return nil
}
Expand Down
Loading