From ada862da650f0417f850a1cf853ba5f62bad3dd7 Mon Sep 17 00:00:00 2001 From: dmiedema Date: Wed, 26 Sep 2018 22:53:41 -0700 Subject: [PATCH] Improve `defaults` command Rename `verboseOutput` to `alwaysVerbose`. It seemed like ELCLI was swallowing commands that had matching prefixes so `--verbose` and `--verboseOutput`` were being captured by the same block when that was not the desired result. Additionally added unit tests for the defaults command around migrations, passing trash values, and its use being applied as expected. --- ModuloKit/Commands/AddCommand.swift | 2 +- ModuloKit/Commands/DefaultsCommand.swift | 28 ++-- ModuloKit/Commands/InitCommand.swift | 2 +- ModuloKit/Commands/MapCommand.swift | 2 +- ModuloKit/Commands/RemoveCommand.swift | 2 +- ModuloKit/Commands/SetCommand.swift | 2 +- ModuloKit/Commands/StatusCommand.swift | 2 +- ModuloKit/Commands/UpdateCommand.swift | 2 +- ModuloKit/Specs/OptionsSpec.swift | 6 +- ModuloKitTests/TestDefaults.swift | 147 +++++++++++++++++- .../xcshareddata/xcschemes/modulo.xcscheme | 6 +- 11 files changed, 177 insertions(+), 24 deletions(-) diff --git a/ModuloKit/Commands/AddCommand.swift b/ModuloKit/Commands/AddCommand.swift index 422b6a6..66de609 100644 --- a/ModuloKit/Commands/AddCommand.swift +++ b/ModuloKit/Commands/AddCommand.swift @@ -31,7 +31,7 @@ open class AddCommand: NSObject, Command { } open var failOnUnrecognizedOptions: Bool { return true } - open var verbose: Bool = State.instance.options.verboseOutput + open var verbose: Bool = State.instance.options.alwaysVerbose open var quiet: Bool = false open func configureOptions() { diff --git a/ModuloKit/Commands/DefaultsCommand.swift b/ModuloKit/Commands/DefaultsCommand.swift index 410af6b..577f0f8 100644 --- a/ModuloKit/Commands/DefaultsCommand.swift +++ b/ModuloKit/Commands/DefaultsCommand.swift @@ -36,7 +36,7 @@ open class DefaultsCommand: NSObject, Command { public var failOnUnrecognizedOptions: Bool { return true } - public var verbose: Bool = State.instance.options.verboseOutput + public var verbose: Bool = State.instance.options.alwaysVerbose public var quiet: Bool = false public func execute(_ otherParams: Array?) -> Int { @@ -58,23 +58,27 @@ open class DefaultsCommand: NSObject, Command { newValue = false } - spec.options.verboseOutput = newValue - State.instance.options.verboseOutput = newValue + spec.options.alwaysVerbose = newValue + State.instance.options.alwaysVerbose = newValue } if let moduleFolderPath = moduleFolderPath, !moduleFolderPath.isEmpty { spec.options.depdencyInstallationPath = moduleFolderPath State.instance.options.depdencyInstallationPath = moduleFolderPath } + + if !toggleVerbose && moduleFolderPath == nil { + writeln(.stderr, """ + When `--set` is passed its assumed you want to set a default. + Please specify one of the options + --alwaysVerbose + --moduleFolder + """) + } spec.save() } else { - if toggleVerbose { - writeln(.stdout, "VerboseOutput - \(spec.options.verboseOutput)") - } - if moduleFolderPath != nil { - writeln(.stdout, "depdencyInstallationPath - \(spec.options.depdencyInstallationPath)") - } - + writeln(.stdout, "alwaysVerbose - \(spec.options.alwaysVerbose)") + writeln(.stdout, "depdencyInstallationPath - \(spec.options.depdencyInstallationPath)") } return ErrorCode.success.rawValue @@ -85,9 +89,9 @@ open class DefaultsCommand: NSObject, Command { self.setValue = true } - addOptionValue(["--verboseOutput"], + addOptionValue(["--alwaysVerbose"], usage: "specify `verbose` for all commands that are run", - valueSignature: "<[true|false}>") { (option, value) in + valueSignature: "<[true|false]>") { (option, value) in self.toggleVerbose = true self.verboseValue = value } diff --git a/ModuloKit/Commands/InitCommand.swift b/ModuloKit/Commands/InitCommand.swift index 2c974b5..91f3299 100644 --- a/ModuloKit/Commands/InitCommand.swift +++ b/ModuloKit/Commands/InitCommand.swift @@ -25,7 +25,7 @@ open class InitCommand: NSObject, Command { } open var failOnUnrecognizedOptions: Bool { return true } - open var verbose: Bool = State.instance.options.verboseOutput + open var verbose: Bool = State.instance.options.alwaysVerbose open var quiet: Bool = false open func configureOptions() { diff --git a/ModuloKit/Commands/MapCommand.swift b/ModuloKit/Commands/MapCommand.swift index 933acc8..c02c663 100644 --- a/ModuloKit/Commands/MapCommand.swift +++ b/ModuloKit/Commands/MapCommand.swift @@ -21,7 +21,7 @@ open class MapCommand: NSObject, Command { } open var failOnUnrecognizedOptions: Bool { return true } - open var verbose: Bool = State.instance.options.verboseOutput + open var verbose: Bool = State.instance.options.alwaysVerbose open var quiet: Bool = false fileprivate var simple = false diff --git a/ModuloKit/Commands/RemoveCommand.swift b/ModuloKit/Commands/RemoveCommand.swift index 4de0887..5a2672c 100644 --- a/ModuloKit/Commands/RemoveCommand.swift +++ b/ModuloKit/Commands/RemoveCommand.swift @@ -21,7 +21,7 @@ open class RemoveCommand: NSObject, Command { } open var failOnUnrecognizedOptions: Bool { return true } - open var verbose: Bool = State.instance.options.verboseOutput + open var verbose: Bool = State.instance.options.alwaysVerbose open var quiet: Bool = false open func configureOptions() { diff --git a/ModuloKit/Commands/SetCommand.swift b/ModuloKit/Commands/SetCommand.swift index 84d7a4c..c2610ef 100644 --- a/ModuloKit/Commands/SetCommand.swift +++ b/ModuloKit/Commands/SetCommand.swift @@ -21,7 +21,7 @@ open class SetCommand: NSObject, Command { } open var failOnUnrecognizedOptions: Bool { return true } - open var verbose: Bool = State.instance.options.verboseOutput + open var verbose: Bool = State.instance.options.alwaysVerbose open var quiet: Bool = false // subcommands diff --git a/ModuloKit/Commands/StatusCommand.swift b/ModuloKit/Commands/StatusCommand.swift index 104af44..3da9045 100644 --- a/ModuloKit/Commands/StatusCommand.swift +++ b/ModuloKit/Commands/StatusCommand.swift @@ -23,7 +23,7 @@ open class StatusCommand: NSObject, Command { open var failOnUnrecognizedOptions: Bool { return true } open var ignoreMain: Bool = false - open var verbose: Bool = State.instance.options.verboseOutput + open var verbose: Bool = State.instance.options.alwaysVerbose open var quiet: Bool = false open func configureOptions() { diff --git a/ModuloKit/Commands/UpdateCommand.swift b/ModuloKit/Commands/UpdateCommand.swift index 393665c..0cef409 100644 --- a/ModuloKit/Commands/UpdateCommand.swift +++ b/ModuloKit/Commands/UpdateCommand.swift @@ -29,7 +29,7 @@ open class UpdateCommand: NSObject, Command { } open var failOnUnrecognizedOptions: Bool { return true } - open var verbose: Bool = State.instance.options.verboseOutput + open var verbose: Bool = State.instance.options.alwaysVerbose open var quiet: Bool = false open func configureOptions() { diff --git a/ModuloKit/Specs/OptionsSpec.swift b/ModuloKit/Specs/OptionsSpec.swift index c8a5ace..701e9d4 100644 --- a/ModuloKit/Specs/OptionsSpec.swift +++ b/ModuloKit/Specs/OptionsSpec.swift @@ -14,7 +14,7 @@ import Foundation public struct OptionsSpec { /// Should we have `verbose` on all commands - var verboseOutput: Bool = false + var alwaysVerbose: Bool = false /// Path to store our 'modules'/dependencies in var depdencyInstallationPath: String = "modules" } @@ -22,7 +22,7 @@ public struct OptionsSpec { extension OptionsSpec: ELDecodable { public static func decode(_ json: JSON?) throws -> OptionsSpec { return try OptionsSpec( - verboseOutput: json ==> "verboseOutput", + alwaysVerbose: json ==> "alwaysVerbose", depdencyInstallationPath: json ==> "depdencyInstallationPath" ) } @@ -31,7 +31,7 @@ extension OptionsSpec: ELDecodable { extension OptionsSpec: ELEncodable { public func encode() throws -> JSON { return try encodeToJSON([ - "verboseOutput" <== verboseOutput, + "alwaysVerbose" <== alwaysVerbose, "depdencyInstallationPath" <== depdencyInstallationPath ]) } diff --git a/ModuloKitTests/TestDefaults.swift b/ModuloKitTests/TestDefaults.swift index 0627466..7436543 100644 --- a/ModuloKitTests/TestDefaults.swift +++ b/ModuloKitTests/TestDefaults.swift @@ -9,9 +9,154 @@ import XCTest import ELCLI import ELFoundation +import ELCodable // for JSON @testable import ModuloKit class TestDefaults: XCTestCase { let modulo = Modulo() - + + // MARK: - Setup + override func setUp() { + super.setUp() + + moduloReset() + } + + // MARK: - Migration + func testMigrationFromNoOptionsInModuloFile() { + let moduleFileJSONDictionary = [ + "dependencies": [], + "module": false, + "name": "best project" + ] as [String : Any] + do { + let moduleSpec = try spec(from: moduleFileJSONDictionary) + let options = moduleSpec.options + // validate defaults + XCTAssertTrue(options.alwaysVerbose == false) + XCTAssertTrue(options.depdencyInstallationPath == "modules") + } catch { + XCTFail("Failed with error \(error)") + } + } + + // MARK: - Bad Input + func testGarbageValuesInModuloFileResultInSaneDefaults() { + let moduleFileJSONDictionary = [ + "dependencies": [], + "module": false, + "name": "best project", + "options": [ + "alwaysVerbose": "lolgarbage", + "depdencyInstallationPath": ["fart": "toot"], + "invalid_key": true, + ] + ] as [String : Any] + do { + let moduleSpec = try spec(from: moduleFileJSONDictionary) + let options = moduleSpec.options + // validate defaults since we fed it garbage + XCTAssertTrue(options.alwaysVerbose == false) + XCTAssertTrue(options.depdencyInstallationPath == "modules") + } catch { + XCTFail("Failed with error \(error)") + } + } + + // MARK: - Loaded from module file + func testOptionsAreParsedFromModuleFile() { + let directoryPath = "only-the-best-dependencies-live-here" + let moduleFileJSONDictionary = [ + "dependencies": [], + "module": false, + "name": "best project", + "options": [ + "alwaysVerbose": true, + "depdencyInstallationPath": directoryPath, + ] + ] as [String : Any] + + do { + let moduleSpec = try spec(from: moduleFileJSONDictionary) + let options = moduleSpec.options + XCTAssertTrue(options.alwaysVerbose == true) + XCTAssertTrue(options.depdencyInstallationPath == directoryPath) + } catch { + XCTFail("Failed with error \(error)") + } + } + + // MARK: - CLI API + func testReadingAllDefaults() { + _ = Modulo() + moduloReset() + + let result = Modulo.run(["defaults"]) + + // ideally we'd capture output (stdout) somehow + // and verify our output is what we want but since + // i can't see a nice way to do that with ELCLI + // we'll just verify success instead. + + XCTAssertTrue(result == .success) + } + + func testSettingDefault() { + _ = Modulo() + moduloReset() + XCTAssertFalse(State.instance.options.alwaysVerbose) + let verboseResult = Modulo.run(["defaults", "--set", "--alwaysVerbose", "true"]) + XCTAssertTrue(verboseResult == .success) + XCTAssertTrue(State.instance.options.alwaysVerbose) + + let directoryResult = Modulo.run(["defaults", "--set", "--moduleFolder", "bestDIR"]) + XCTAssertTrue(directoryResult == .success) + XCTAssertTrue(State.instance.options.depdencyInstallationPath == "bestDIR") + } + + func testFailsIfSettingBadDefault() { + _ = Modulo() + moduloReset() + + xctAssertThrows({ + _ = Modulo.run(["defaults", "--set", "--NoTheRightValue", "badValue.BadMe."]) + }, "Running `defaults --set` with a bad flag/value did not fail") + } + + func testSettingDefaultWithBadValue() { + _ = Modulo() + moduloReset() + State.instance.options.alwaysVerbose = true + + let badVerboseResult = Modulo.run(["defaults", "--set", "--alwaysVerbose", "ohSoVerbose"]) + XCTAssertTrue(badVerboseResult == .success) + XCTAssertFalse(State.instance.options.alwaysVerbose) + } + + func testSettingWithNoKey() { + _ = Modulo() + moduloReset() + + let initialVerbose = State.instance.options.alwaysVerbose + let initialPath = State.instance.options.depdencyInstallationPath + let result = Modulo.run(["defaults", "--set"]) + XCTAssertTrue(result == .success, "Even though we set nothing, we shoud succeed with some output for the user") + XCTAssertTrue(initialVerbose == State.instance.options.alwaysVerbose) + XCTAssertTrue(initialPath == State.instance.options.depdencyInstallationPath) + } +} + +// MARK: - Test Helpers +extension TestDefaults { + func spec(from dictionary: [String: Any]) throws -> ModuleSpec { + let moduleFileJSONData = try JSONSerialization.data(withJSONObject: dictionary, options: .prettyPrinted) + guard let moduleFileJSON = JSON(data: moduleFileJSONData) else { + throw NSError(domain: "TestDefaults", + code: -1, + userInfo: [ + NSLocalizedDescriptionKey: "Failed to create JSON from Data" + ]) + } + return try ModuleSpec.decode(moduleFileJSON) + } } diff --git a/modulo.xcodeproj/xcshareddata/xcschemes/modulo.xcscheme b/modulo.xcodeproj/xcshareddata/xcschemes/modulo.xcscheme index 7faccf9..b00a1fb 100644 --- a/modulo.xcodeproj/xcshareddata/xcschemes/modulo.xcscheme +++ b/modulo.xcodeproj/xcshareddata/xcschemes/modulo.xcscheme @@ -67,13 +67,17 @@ argument = "update --all --host github.corp.ebay.com" isEnabled = "NO"> + + + isEnabled = "NO">