Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dataset validation for sync push command #241

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
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
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,20 @@ struct TestFlightProgramDifference {
}
}

enum Error: LocalizedError, Equatable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an argument to be made around not adding equatable conformance in the implementation if you only use it for testing. Not sure how I feel about it though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an argument to be made around not adding equatable conformance in the implementation if you only use it for testing. Not sure how I feel about it though

Any ideas for removing the Equatable in this case? 🤔

Copy link
Contributor

@adamjcampbell adamjcampbell Oct 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you wanted to you could replace the instances in testing with something like:

XCTAssertThrowsError(try TestFlightProgramDifference(local: local, remote: remote)) { error in
  guard case TestFlightProgramDifference.Error.duplicateTesters(let email) = error else {
    return XCTFail()
  }

  XCTAssertEqual(email, "[email protected]")
}

case duplicateTesters(email: String)

var errorDescription: String? {
switch self {
case .duplicateTesters(let email):
return "There are two beta testers with the same email '\(email)' exists in your account, please clean that up before continue."
}
}
}

let changes: [Change]

init(local: TestFlightProgram, remote: TestFlightProgram) {
init(local: TestFlightProgram, remote: TestFlightProgram) throws {
var changes: [Change] = []

// Groups
Expand All @@ -80,6 +91,10 @@ struct TestFlightProgramDifference {
let remoteApps = remoteTester.apps
let remoteBetaGroups = remoteTester.betaGroups

if remote.testers.filter({ $0.email == remoteTester.email}).count > 1 {
throw Error.duplicateTesters(email: remoteTester.email ?? "")
}

if let localTester = local.testers.first(where: { $0.email == remoteTester.email }) {
let appsToAdd = localTester.apps.filter { app in
let appIds = remoteApps.map(\.id) + remoteBetaGroups.compactMap(\.app?.id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ struct TestFlightPushCommand: CommonParsableCommand {
let local = try FileSystem.readTestFlightConfiguration(from: inputPath)
let remote = try service.getTestFlightProgram()

let difference = TestFlightProgramDifference(local: local, remote: remote)
let difference = try TestFlightProgramDifference(local: local, remote: remote)

difference.changes.forEach { print($0.description) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,25 @@ struct TestFlightConfigurationProcessor {

enum Error: LocalizedError {
case testerNotInTestersList(email: String, betaGroup: BetaGroup, app: App)
case noValidApp

var errorDescription: String? {
switch self {
case .testerNotInTestersList(let email, let betaGroup, let app):
return "Tester with email: \(email) in beta group named: \(betaGroup.groupName) " +
"for app: \(app.bundleId) is not included in the \(betaTestersCSVName) file"

case .noValidApp:
return "There's no valid app folder found in your local configuration file path, please run 'sync pull' first"
}
}
}

func readConfiguration() throws -> TestFlightConfiguration {
let folder = try Folder(path: path)

guard folder.subfolders.count() > 0 else { throw Error.noValidApp }

var configuration = TestFlightConfiguration()

let decodeBetaTesters: (Data) throws -> [BetaTester] = { data in
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright 2020 Itty Bitty Apps Pty Ltd

@testable import AppStoreConnectCLI

import FileSystem
import Model
import Foundation
import XCTest

final class TestFlightProgramDifferenceTests: XCTestCase {

func testErrorWillBeThrow_whenDuplicateTesters() throws {
let local = TestFlightProgram(
apps: [],
testers: [BetaTester(email: "[email protected]", firstName: "Foo", lastName: "Bar", inviteType: "EMAIL", betaGroups: [], apps: [])],
groups: []
)

let remote = TestFlightProgram(
apps: [],
testers: [
BetaTester(email: "[email protected]", firstName: "Foo", lastName: "Bar", inviteType: "EMAIL", betaGroups: [], apps: []),
BetaTester(email: "[email protected]", firstName: "Foo", lastName: "Bar", inviteType: "EMAIL", betaGroups: [], apps: []),
],
groups: []
)

XCTAssertThrowsError(try TestFlightProgramDifference(local: local, remote: remote)) { error in
XCTAssertEqual(
error as? TestFlightProgramDifference.Error,
TestFlightProgramDifference.Error.duplicateTesters(email: "[email protected]")
)
}
}

func testErrorNotThrow_withoutDuplicateTesters() throws {
let local = TestFlightProgram(
apps: [],
testers: [BetaTester(email: "[email protected]", firstName: "Foo", lastName: "Bar", inviteType: "EMAIL", betaGroups: [], apps: [])],
groups: []
)

let remote = TestFlightProgram(
apps: [],
testers: [BetaTester(email: "[email protected]", firstName: "Foo", lastName: "Bar", inviteType: "EMAIL", betaGroups: [], apps: [])],
groups: []
)

let result = try TestFlightProgramDifference(local: local, remote: remote)

XCTAssertTrue(result.changes.isEmpty)
}

}