From 52ad422f7615823053b787063f910a57f659719c Mon Sep 17 00:00:00 2001 From: Peter Finn Date: Tue, 5 Nov 2019 18:44:28 -0800 Subject: [PATCH 1/6] return octo.exe result and marks trace success correctly --- src/app/Fake.Tools.Octo/Octo.fs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/app/Fake.Tools.Octo/Octo.fs b/src/app/Fake.Tools.Octo/Octo.fs index 898346ff607..21fd1fd81db 100644 --- a/src/app/Fake.Tools.Octo/Octo.fs +++ b/src/app/Fake.Tools.Octo/Octo.fs @@ -261,7 +261,7 @@ let private exec command options = let commandString = command.ToString() - use __ = Trace.traceTask "Octo "commandString + use __ = Trace.traceTask "Octo " commandString Trace.trace (tool + traceArgs) let result = @@ -274,8 +274,13 @@ let private exec command options = ) options.Timeout match result with - | 0 -> () - | _ -> failwithf "Octo %s failed. Process finished with exit code %i" commandString result + | 0 -> + __.MarkSuccess() + result + | _ -> + __.MarkFailed() + failwithf "Octo %s failed. Process finished with exit code %i" commandString result + result /// Creates a release. let createRelease setParams = From 9133d1bfba9f7051aff48653a89410ebc2f71e50 Mon Sep 17 00:00:00 2001 From: Peter Finn Date: Wed, 6 Nov 2019 09:54:02 -0800 Subject: [PATCH 2/6] adds functions to return exit code --- src/app/Fake.Tools.Octo/Octo.fs | 44 +++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/app/Fake.Tools.Octo/Octo.fs b/src/app/Fake.Tools.Octo/Octo.fs index 21fd1fd81db..53a37c821af 100644 --- a/src/app/Fake.Tools.Octo/Octo.fs +++ b/src/app/Fake.Tools.Octo/Octo.fs @@ -251,7 +251,6 @@ let private commandLine command = sprintf " push%s" (pushCommandLine opts) let private exec command options = - let serverCommandLineForTracing (opts: ServerOptions) = serverCommandLine { opts with ApiKey = "(Removed for security purposes)" } @@ -282,33 +281,52 @@ let private exec command options = failwithf "Octo %s failed. Process finished with exit code %i" commandString result result -/// Creates a release. -let createRelease setParams = +/// Creates a release and returns the exit code. +let createReleaseWithExitCode setParams = let options = setParams releaseOptions exec (CreateRelease (options, None)) options.Common -/// Creates a release, and optionally deploys it to one or more environments. -let createReleaseAndDeploy setReleaseParams setDeployParams = +/// Creates a release, and optionally deploys it to one or more environments and returns the exit code. +let createReleaseAndDeployWithExitCode setReleaseParams setDeployParams = let releaseOptions = setReleaseParams releaseOptions let deployOptions = setDeployParams deployOptions exec (CreateRelease (releaseOptions, deployOptions)) releaseOptions.Common -/// Deploys releases that have already been created. -let deployRelease setParams = +/// Deploys releases that have already been created and returns the exit code. +let deployReleaseWithExitCode setParams = let options = setParams deployOptions exec (DeployRelease options) options.Common -/// Deletes a range of releases. -let deleteReleases setParams = +/// Deletes a range of releases and returns the exit code. +let deleteReleasesWithExitCode setParams = let options = setParams deleteOptions exec (DeleteReleases options) options.Common -/// Lists all environments. -let listEnvironments setParams = +/// Lists all environments and returns the exit code. +let listEnvironmentsWithExitCode setParams = let options = setParams commonOptions exec ListEnvironments options -/// Pushes one or more packages to the Octopus built-in repository. -let push setParams = +/// Pushes one or more packages to the Octopus built-in repository and returns the exit code. +let pushWithExitCode setParams = let options = setParams pushOptions exec (Push options) options.Common + +/// Creates a release. +let createRelease = createReleaseWithExitCode |> ignore + +/// Creates a release, and optionally deploys it to one or more environments. +let createReleaseAndDeploy = createReleaseAndDeployWithExitCode |> ignore + +/// Deploys releases that have already been created. +let deployRelease = deployReleaseWithExitCode |> ignore + +/// Deletes a range of releases. +let deleteReleases = deleteReleasesWithExitCode |> ignore + +/// Lists all environments. +let listEnvironments = listEnvironmentsWithExitCode |> ignore + +/// Pushes one or more packages to the Octopus built-in repository. +let push = pushWithExitCode |> ignore + From 1b086c0286ec3a383e1b4869c63980948308bbfc Mon Sep 17 00:00:00 2001 From: Peter Finn Date: Thu, 7 Nov 2019 13:20:20 -0800 Subject: [PATCH 3/6] readd parameters instead of partial application forgot how partial application works and that it is not just magic. also forgot ignore is a function. --- src/app/Fake.Tools.Octo/Octo.fs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/app/Fake.Tools.Octo/Octo.fs b/src/app/Fake.Tools.Octo/Octo.fs index 53a37c821af..1b020de889d 100644 --- a/src/app/Fake.Tools.Octo/Octo.fs +++ b/src/app/Fake.Tools.Octo/Octo.fs @@ -313,20 +313,20 @@ let pushWithExitCode setParams = exec (Push options) options.Common /// Creates a release. -let createRelease = createReleaseWithExitCode |> ignore +let createRelease setParams = createReleaseWithExitCode setParams |> ignore /// Creates a release, and optionally deploys it to one or more environments. -let createReleaseAndDeploy = createReleaseAndDeployWithExitCode |> ignore +let createReleaseAndDeploy setReleaseParams setDeployParams = createReleaseAndDeployWithExitCode setReleaseParams setDeployParams |> ignore /// Deploys releases that have already been created. -let deployRelease = deployReleaseWithExitCode |> ignore +let deployRelease setParams = deployReleaseWithExitCode setParams |> ignore /// Deletes a range of releases. -let deleteReleases = deleteReleasesWithExitCode |> ignore +let deleteReleases setParams = deleteReleasesWithExitCode setParams |> ignore /// Lists all environments. -let listEnvironments = listEnvironmentsWithExitCode |> ignore +let listEnvironments setParams = listEnvironmentsWithExitCode setParams |> ignore /// Pushes one or more packages to the Octopus built-in repository. -let push = pushWithExitCode |> ignore +let push setParams = pushWithExitCode setParams |> ignore From 22e81a2b336a5a8ccde4e58ec2c89b72db2f2e49 Mon Sep 17 00:00:00 2001 From: Peter Finn Date: Thu, 7 Nov 2019 18:10:28 -0800 Subject: [PATCH 4/6] Using CreateProcess instead of execSimple --- src/app/Fake.Tools.Octo/Octo.fs | 56 ++++++++++++++++----------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/src/app/Fake.Tools.Octo/Octo.fs b/src/app/Fake.Tools.Octo/Octo.fs index 1b020de889d..d335d71958d 100644 --- a/src/app/Fake.Tools.Octo/Octo.fs +++ b/src/app/Fake.Tools.Octo/Octo.fs @@ -174,18 +174,18 @@ let private pushOptions = { let private optionalStringParam p o = match o with - | Some s -> sprintf " --%s=\"%s\"" p s + | Some s -> sprintf "--%s=%s" p s | None -> "" let private optionalObjParam p o = match o with - | Some x -> sprintf " --%s=\"%s\"" p (x.ToString()) + | Some x -> sprintf "--%s=%s" p (x.ToString()) | None -> "" let private stringListParam p os = let sb = Text.StringBuilder() for o in os do - sb.Append (sprintf " --%s=\"%s\"" p (o.ToString())) |> ignore + sb.Append (sprintf " --%s=%s" p (o.ToString())) |> ignore sb.ToString() let private flag p b = if b then sprintf " --%s" p else "" @@ -200,9 +200,9 @@ let private releaseCommandLine (opts:CreateReleaseOptions) = (optionalStringParam "releasenotesfile" (String.liftString opts.ReleaseNotesFile)) (flag "ignoreExisting" opts.IgnoreExisting) (optionalStringParam "channel" opts.Channel) - (flag "ignorechannelrules" opts.IgnoreChannelRules) ] - |> List.fold (+) "" - + (flag "ignorechannelrules" opts.IgnoreChannelRules) ] + |> List.filter String.isNotNullOrEmpty + let private deployCommandLine (opts:DeployReleaseOptions) = [ (optionalStringParam "project" (String.liftString opts.Project)) (optionalStringParam "deployto" (String.liftString opts.DeployTo)) @@ -214,49 +214,50 @@ let private deployCommandLine (opts:DeployReleaseOptions) = (optionalObjParam "deploymenttimeout" opts.DeploymentTimeout) (optionalObjParam "deploymentchecksleepcycle" opts.DeploymentCheckSleepCycle) (optionalStringParam "specificmachines" opts.SpecificMachines) - (optionalStringParam "channel" opts.Channel) ] - |> List.fold (+) "" + (optionalStringParam "channel" opts.Channel) ] + |> List.filter String.isNotNullOrEmpty let private deleteCommandLine (opts:DeleteReleasesOptions) = [ (optionalStringParam "project" (String.liftString opts.Project)) (optionalStringParam "minversion" (String.liftString opts.MinVersion)) (optionalStringParam "maxversion" (String.liftString opts.MaxVersion)) - (optionalStringParam "channel" (opts.Channel)) ] - |> List.fold (+) "" + (optionalStringParam "channel" (opts.Channel)) ] + |> List.filter String.isNotNullOrEmpty let private serverCommandLine (opts:ServerOptions) = [ (optionalStringParam "server" (String.liftString opts.ServerUrl)) - (optionalStringParam "apikey" (String.liftString opts.ApiKey)) ] - |> List.fold (+) "" + (optionalStringParam "apikey" (String.liftString opts.ApiKey)) ] + |> List.filter String.isNotNullOrEmpty let private pushCommandLine (opts : PushOptions) = [ stringListParam "package" opts.Packages flag "replace-existing" opts.ReplaceExisting ] - |> List.fold (+) "" + |> List.filter String.isNotNullOrEmpty /// Maps a command to string input for the octopus tools cli. let private commandLine command = match command with | CreateRelease (opts, None) -> - sprintf " create-release%s" (releaseCommandLine opts) + "create-release" :: (releaseCommandLine opts) | CreateRelease (opts, Some (dopts)) -> - sprintf " create-release%s%s" (releaseCommandLine opts) (deployCommandLine dopts) + "create-release" :: ( List.append (releaseCommandLine opts) (deployCommandLine dopts) ) | DeployRelease opts -> - sprintf " deploy-release%s" (deployCommandLine opts) + "deploy-release" :: (deployCommandLine opts) | DeleteReleases opts -> - sprintf " delete-releases%s" (deleteCommandLine opts) + "delete-releases" :: (deleteCommandLine opts) | ListEnvironments -> - " list-environments" + ["list-environments"] | Push opts -> - sprintf " push%s" (pushCommandLine opts) + "push" :: (pushCommandLine opts) let private exec command options = let serverCommandLineForTracing (opts: ServerOptions) = serverCommandLine { opts with ApiKey = "(Removed for security purposes)" } let tool = options.ToolPath @@ options.ToolName - let args = commandLine command |>(+)<| serverCommandLine options.Server - let traceArgs = commandLine command |>(+)<| serverCommandLineForTracing options.Server + let args = List.append (commandLine command) (serverCommandLine options.Server) + |> Arguments.OfArgs + let traceArgs = (List.append (commandLine command) (serverCommandLineForTracing options.Server)) |> List.fold (+) "" let commandString = command.ToString() @@ -264,13 +265,12 @@ let private exec command options = Trace.trace (tool + traceArgs) let result = - Process.execSimple (fun info -> - {info with - Arguments = args - WorkingDirectory = options.WorkingDirectory - FileName = tool - } - ) options.Timeout + RawCommand (tool, args) + |> CreateProcess.fromCommand + |> CreateProcess.withWorkingDirectory options.WorkingDirectory + |> CreateProcess.withTimeout options.Timeout + |> Proc.run + |> (fun finishedProccess -> finishedProccess.ExitCode) match result with | 0 -> From fb882b7c8b5c610f828310e46144e53e54368e4d Mon Sep 17 00:00:00 2001 From: Peter Finn Date: Mon, 11 Nov 2019 17:03:56 -0800 Subject: [PATCH 5/6] Adds test and moves exception to no exit code flow --- src/app/Fake.Tools.Octo/Octo.fs | 58 ++++++++++++++----- .../Fake.Core.UnitTests.fsproj | 2 + .../Fake.Tools.Testing.Octo.fs | 53 +++++++++++++++++ 3 files changed, 98 insertions(+), 15 deletions(-) create mode 100644 src/test/Fake.Core.UnitTests/Fake.Tools.Testing.Octo.fs diff --git a/src/app/Fake.Tools.Octo/Octo.fs b/src/app/Fake.Tools.Octo/Octo.fs index d335d71958d..bed75b40ff0 100644 --- a/src/app/Fake.Tools.Octo/Octo.fs +++ b/src/app/Fake.Tools.Octo/Octo.fs @@ -7,6 +7,10 @@ open Fake.IO.Globbing open Fake.IO.FileSystemOperators open System open System.IO +open System.Runtime.CompilerServices + +[] +do() /// Octo.exe server options type ServerOptions = { @@ -133,7 +137,7 @@ type PushOptions = { Common: Options} /// Option type for selecting one command -type private Command = +type internal Command = | CreateRelease of CreateReleaseOptions * DeployReleaseOptions option | DeployRelease of DeployReleaseOptions | DeleteReleases of DeleteReleasesOptions @@ -144,7 +148,7 @@ type private Command = let private serverOptions = { ServerUrl = ""; ApiKey = ""; } /// Default parameters to call octo.exe. -let private commonOptions = +let internal commonOptions = let toolName = "Octo.exe" { ToolPath = Tools.findToolFolderInSubPath toolName (Directory.GetCurrentDirectory() @@ "tools" @@ "OctopusTools") ToolName = toolName @@ -153,23 +157,23 @@ let private commonOptions = WorkingDirectory = "" } /// Default options for 'CreateRelease' -let private releaseOptions = { +let internal releaseOptions = { Project = ""; Version = ""; PackageVersion = ""; Packages = []; PackagesFolder = None; ReleaseNotes = ""; ReleaseNotesFile = ""; IgnoreExisting = false; Channel = None; IgnoreChannelRules = false; Common = commonOptions} /// Default options for 'DeployRelease' -let private deployOptions = { +let internal deployOptions = { Project = ""; DeployTo = ""; Version = ""; Force = false; WaitForDeployment = false; DeploymentTimeout = None; DeploymentCheckSleepCycle = None; SpecificMachines = None; NoRawLog = false; Progress = false; Channel = None; Common = commonOptions } /// Default options for 'DeleteReleases' -let private deleteOptions = { +let internal deleteOptions = { Project = ""; MinVersion = ""; MaxVersion = ""; Channel = None; Common = commonOptions } /// Default options for 'Push' -let private pushOptions = { +let internal pushOptions = { Packages = []; ReplaceExisting = false; Common = commonOptions} let private optionalStringParam p o = @@ -235,7 +239,7 @@ let private pushCommandLine (opts : PushOptions) = |> List.filter String.isNotNullOrEmpty /// Maps a command to string input for the octopus tools cli. -let private commandLine command = +let internal commandLine command = match command with | CreateRelease (opts, None) -> "create-release" :: (releaseCommandLine opts) @@ -278,7 +282,6 @@ let private exec command options = result | _ -> __.MarkFailed() - failwithf "Octo %s failed. Process finished with exit code %i" commandString result result /// Creates a release and returns the exit code. @@ -312,21 +315,46 @@ let pushWithExitCode setParams = let options = setParams pushOptions exec (Push options) options.Common + +let private handelIgnoreExitCode commandString result = + match result with + | 0 -> + () + | _ -> + failwithf "Octo %s failed. Process finished with exit code %i" commandString result + /// Creates a release. -let createRelease setParams = createReleaseWithExitCode setParams |> ignore +let createRelease setParams = + let commandLine = (CreateRelease ((setParams releaseOptions), None)).ToString() + createReleaseWithExitCode setParams + |> (handelIgnoreExitCode <| commandLine) /// Creates a release, and optionally deploys it to one or more environments. -let createReleaseAndDeploy setReleaseParams setDeployParams = createReleaseAndDeployWithExitCode setReleaseParams setDeployParams |> ignore +let createReleaseAndDeploy setReleaseParams setDeployParams = + let commandLine = (CreateRelease ((setReleaseParams releaseOptions), (setDeployParams deployOptions))).ToString() + createReleaseAndDeployWithExitCode setReleaseParams setDeployParams + |> (handelIgnoreExitCode <| commandLine ) /// Deploys releases that have already been created. -let deployRelease setParams = deployReleaseWithExitCode setParams |> ignore +let deployRelease setParams = + let commandLine = (DeployRelease (setParams deployOptions)).ToString() + deployReleaseWithExitCode setParams + |> (handelIgnoreExitCode <| commandLine ) /// Deletes a range of releases. -let deleteReleases setParams = deleteReleasesWithExitCode setParams |> ignore +let deleteReleases setParams = + let commandLine = (DeleteReleases ((setParams deleteOptions))).ToString() + deleteReleasesWithExitCode setParams + |> (handelIgnoreExitCode <| commandLine ) /// Lists all environments. -let listEnvironments setParams = listEnvironmentsWithExitCode setParams |> ignore +let listEnvironments setParams = + let commandLine = (ListEnvironments).ToString() + listEnvironmentsWithExitCode setParams + |> (handelIgnoreExitCode <| commandLine) /// Pushes one or more packages to the Octopus built-in repository. -let push setParams = pushWithExitCode setParams |> ignore - +let push setParams = + let commandLine = (Push ((setParams pushOptions))).ToString() + pushWithExitCode setParams + |> (handelIgnoreExitCode <| commandLine) diff --git a/src/test/Fake.Core.UnitTests/Fake.Core.UnitTests.fsproj b/src/test/Fake.Core.UnitTests/Fake.Core.UnitTests.fsproj index 4f6f55c4fd6..e5ea90e0976 100644 --- a/src/test/Fake.Core.UnitTests/Fake.Core.UnitTests.fsproj +++ b/src/test/Fake.Core.UnitTests/Fake.Core.UnitTests.fsproj @@ -32,6 +32,7 @@ + @@ -62,6 +63,7 @@ + diff --git a/src/test/Fake.Core.UnitTests/Fake.Tools.Testing.Octo.fs b/src/test/Fake.Core.UnitTests/Fake.Tools.Testing.Octo.fs new file mode 100644 index 00000000000..1898911f2e2 --- /dev/null +++ b/src/test/Fake.Core.UnitTests/Fake.Tools.Testing.Octo.fs @@ -0,0 +1,53 @@ +module Fake.Tools.Testing.Octo + +open Expecto + +[] +let defaultTests = + testList "Fake.Tools.Octo Tests" [ + testCase "Create Release Default" <| fun _ -> + let expectedCommand = [ + "create-release" + ] + let actual = + (Fake.Tools.Octo.releaseOptions, None) + |> Fake.Tools.Octo.Command.CreateRelease + |> Fake.Tools.Octo.commandLine + Expect.hasLength actual expectedCommand.Length "With default options only expect the command" + Expect.sequenceEqual actual expectedCommand "CreateRelease command should be the create-release string" + + testCase "Create Release Fully Filled Out" <| fun _ -> + let expectedCommand = [ + "create-release" + "--project=Project-1" + "--version=Version-1" + "--packageversion=PackageVersion-1" + " --package=Package-1 --package=Package-2" + "--packagesfolder=PackageFolder-1" + "--releasenotes=ReleaseNotes-1" + "--releasenotesfile=ReleaseNotesFile-1" + " --ignoreExisting" + "--channel=Channel-1" + " --ignorechannelrules" + ] + let releaseOptions = { + Fake.Tools.Octo.releaseOptions with + Project="Project-1" + Version="Version-1" + PackageVersion="PackageVersion-1" + Packages=["Package-1"; "Package-2"] + PackagesFolder=Some "PackageFolder-1" + ReleaseNotes="ReleaseNotes-1" + ReleaseNotesFile="ReleaseNotesFile-1" + IgnoreExisting=true + Channel=Some "Channel-1" + Common=Fake.Tools.Octo.commonOptions + IgnoreChannelRules=true + } + let actual = + (releaseOptions, None) + |> Fake.Tools.Octo.Command.CreateRelease + |> Fake.Tools.Octo.commandLine + Expect.hasLength actual expectedCommand.Length "With default options only expect the command" + Expect.sequenceEqual actual expectedCommand "CreateRelease command should be the create-release string" + ] From 9d450cc66f2eb68e1608d97b46d53ad760059152 Mon Sep 17 00:00:00 2001 From: Peter Finn Date: Mon, 11 Nov 2019 17:20:17 -0800 Subject: [PATCH 6/6] Incorrect spelling. handel -> handle --- src/app/Fake.Tools.Octo/Octo.fs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/app/Fake.Tools.Octo/Octo.fs b/src/app/Fake.Tools.Octo/Octo.fs index bed75b40ff0..246278386ef 100644 --- a/src/app/Fake.Tools.Octo/Octo.fs +++ b/src/app/Fake.Tools.Octo/Octo.fs @@ -316,7 +316,7 @@ let pushWithExitCode setParams = exec (Push options) options.Common -let private handelIgnoreExitCode commandString result = +let private handleIgnoreExitCode commandString result = match result with | 0 -> () @@ -327,34 +327,34 @@ let private handelIgnoreExitCode commandString result = let createRelease setParams = let commandLine = (CreateRelease ((setParams releaseOptions), None)).ToString() createReleaseWithExitCode setParams - |> (handelIgnoreExitCode <| commandLine) + |> (handleIgnoreExitCode <| commandLine) /// Creates a release, and optionally deploys it to one or more environments. let createReleaseAndDeploy setReleaseParams setDeployParams = let commandLine = (CreateRelease ((setReleaseParams releaseOptions), (setDeployParams deployOptions))).ToString() createReleaseAndDeployWithExitCode setReleaseParams setDeployParams - |> (handelIgnoreExitCode <| commandLine ) + |> (handleIgnoreExitCode <| commandLine ) /// Deploys releases that have already been created. let deployRelease setParams = let commandLine = (DeployRelease (setParams deployOptions)).ToString() deployReleaseWithExitCode setParams - |> (handelIgnoreExitCode <| commandLine ) + |> (handleIgnoreExitCode <| commandLine ) /// Deletes a range of releases. let deleteReleases setParams = let commandLine = (DeleteReleases ((setParams deleteOptions))).ToString() deleteReleasesWithExitCode setParams - |> (handelIgnoreExitCode <| commandLine ) + |> (handleIgnoreExitCode <| commandLine ) /// Lists all environments. let listEnvironments setParams = let commandLine = (ListEnvironments).ToString() listEnvironmentsWithExitCode setParams - |> (handelIgnoreExitCode <| commandLine) + |> (handleIgnoreExitCode <| commandLine) /// Pushes one or more packages to the Octopus built-in repository. let push setParams = let commandLine = (Push ((setParams pushOptions))).ToString() pushWithExitCode setParams - |> (handelIgnoreExitCode <| commandLine) + |> (handleIgnoreExitCode <| commandLine)