Skip to content

Commit

Permalink
feat: basic support for release creation
Browse files Browse the repository at this point in the history 
feat: allow selection of space on command line
feat: --no-prompt flag on the command line to force automation mode
fix: flag alias for --output-format and --outputFormat didn't work
fix: Activity spinner was showing over the top of the space selection prompt in interactive mode
  • Loading branch information
@borland
borland authored Aug 1, 2022
1 parent 5354118 commit 0d42bc0
Show file tree
Hide file tree
Showing 19 changed files with 1,046 additions and 244 deletions.
15 changes: 15 additions & 0 deletions cmd/octopus/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package main

import (
"fmt"
"github.com/OctopusDeploy/cli/pkg/constants"
"github.com/spf13/cobra"
"os"
"time"

Expand Down Expand Up @@ -35,6 +37,19 @@ func main() {
cmd.SilenceUsage = true
cmd.SilenceErrors = true

// if we attempt to check the flags before Execute is called, cobra hasn't parsed anything yet,
// so we'll get bad values. PersistentPreRun is a convenient callback for setting up our
// environment after parsing but before execution.
cmd.PersistentPreRun = func(_ *cobra.Command, args []string) {
if noPrompt, err := cmd.PersistentFlags().GetBool(constants.FlagNoPrompt); err == nil && noPrompt {
f.SetPromptDisabled()
}

if spaceNameOrId, err := cmd.PersistentFlags().GetString(constants.FlagSpace); err == nil && spaceNameOrId != "" {
clientFactory.SetSpaceNameOrId(spaceNameOrId)
}
}

if err := cmd.Execute(); err != nil {
cmd.PrintErr(err)
cmd.Println()
Expand Down
199 changes: 135 additions & 64 deletions pkg/apiclient/apiclient_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import (
"github.com/AlecAivazis/survey/v2"
"github.com/OctopusDeploy/cli/pkg/apiclient"
"github.com/OctopusDeploy/cli/test/testutil"
"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/resources"
"github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/spaces"
"github.com/stretchr/testify/assert"
)

const PlaceholderApiKey = "API-XXXXXXXXXXXXXXXXXXXXXXXXXXXXX"
const serverUrl = "http://server"
const placeholderApiKey = "API-XXXXXXXXXXXXXXXXXXXXXXXXXXXXX"

func TestClient_GetSystemClient(t *testing.T) {
httpClient := testutil.NewMockHttpClient()
Expand Down Expand Up @@ -43,68 +43,101 @@ func TestClient_GetSystemClient(t *testing.T) {
}

func TestClient_GetSpacedClient_NoPrompt(t *testing.T) {
t.Run("GetSpacedClient returns an error when no space is specified", func(t *testing.T) {
integrationsSpace := spaces.NewSpace("Integrations")
integrationsSpace.ID = "Spaces-7"

cloudSpace := spaces.NewSpace("Cloud")
cloudSpace.ID = "Spaces-39"

spaceNotSpecifiedMessage := "space must be specified when not running interactively; please set the OCTOPUS_SPACE environment variable or specify --space on the command line"

t.Run("GetSpacedClient returns an error when no space is specified and only one space exists", func(t *testing.T) {
// this would pass in interactive mode; we'd auto select the space, however we don't want to do
// that in no-prompt mode because otherwise people could write a CI script that worked due to
// auto-selection of the first space, which would then unexpectedly break later if someone added a
// second space to the octopus server
rt := testutil.NewFakeApiResponder()
testutil.EnqueueRootResponder(rt)
testutil.EnqueueRootResponder(rt) // even though the config is invalid it still hits /api to check auth, etc

rt.EnqueueResponder("GET", "/api/spaces", func(r *http.Request) (any, error) {
return resources.Resources[*spaces.Space]{Items: []*spaces.Space{
spaces.NewSpace("Integrations"),
}}, nil
})
factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", placeholderApiKey, "", nil)

factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", PlaceholderApiKey, "", nil)
apiClient, err := factory2.GetSpacedClient()
assert.Nil(t, apiClient)
assert.Equal(t, spaceNotSpecifiedMessage, err.Error()) // some strongly-typed errors would probably be nicer
assert.Equal(t, 0, rt.RemainingQueueLength())
})

t.Run("GetSpacedClient returns an error when no space is specified and more than one space exists", func(t *testing.T) {
rt := testutil.NewFakeApiResponder()
testutil.EnqueueRootResponder(rt) // even though the config is invalid it still hits /api to check auth, etc

factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), serverUrl, placeholderApiKey, "", nil)

apiClient, err := factory2.GetSpacedClient()
assert.Nil(t, apiClient)
assert.Equal(t, "cannot use specified space ''. Error: cannot find the item", err.Error()) // some strongly-typed errors would probably be nicer
assert.Equal(t, spaceNotSpecifiedMessage, err.Error()) // some strongly-typed errors would probably be nicer
assert.Equal(t, 0, rt.RemainingQueueLength())
})

t.Run("GetSpacedClient returns an error when a space with the wrong name is specified", func(t *testing.T) {
rt := testutil.NewFakeApiResponder()
testutil.EnqueueRootResponder(rt)

// first it guesses that we might have a space ID
rt.EnqueueRawResponder("GET", "/api/spaces/Integrations", func(r *http.Request) (*http.Response, error) {
return &http.Response{StatusCode: 404}, nil
})

// then it tries a partial name search
rt.EnqueueResponder("GET", "/api/spaces?partialName=Integrations", func(r *http.Request) (any, error) {
return resources.Resources[*spaces.Space]{Items: []*spaces.Space{
spaces.NewSpace("NotIntegrations"),
}}, nil
rt.EnqueueResponder("GET", "/api/spaces/all", func(r *http.Request) (any, error) {
return []*spaces.Space{cloudSpace}, nil
})

factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", PlaceholderApiKey, "Integrations", nil)
factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", placeholderApiKey, "Integrations", nil)

apiClient, err := factory2.GetSpacedClient()
assert.Nil(t, apiClient)
assert.Equal(t, "cannot use specified space 'Integrations'. Error: cannot find the item", err.Error()) // some strongly-typed errors would probably be nicer
assert.Equal(t, "cannot find space 'Integrations'", err.Error()) // some strongly-typed errors would probably be nicer
assert.Equal(t, 0, rt.RemainingQueueLength())
})

t.Run("GetSpacedClient works when the Space ID is directly specified", func(t *testing.T) {
rt := testutil.NewFakeApiResponder()

space7responder := func(r *http.Request) (any, error) {
space7 := spaces.NewSpace("Integrations")
space7.ID = "Spaces-7"
return space7, nil
}
testutil.EnqueueRootResponder(rt)

rt.EnqueueResponder("GET", "/api/spaces/all", func(r *http.Request) (any, error) {
return []*spaces.Space{integrationsSpace}, nil
})

// we need to enqueue this again because after it finds Spaces-7 it will recreate the client and reload the root.
testutil.EnqueueRootResponder(rt)

rt.EnqueueResponder("GET", "/api/spaces/Spaces-7", space7responder)
// note it just goes for /api/Spaces-7 this time
rt.EnqueueResponder("GET", "/api/Spaces-7", func(r *http.Request) (any, error) {
return integrationsSpace, nil
})

factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", placeholderApiKey, "Spaces-7", nil)

apiClient, err := factory2.GetSpacedClient()
assert.Nil(t, err)
assert.NotNil(t, apiClient)
assert.Equal(t, 0, rt.RemainingQueueLength())
})

t.Run("GetSpacedClient works when the Space ID is directly specified (case insensitive)", func(t *testing.T) {
rt := testutil.NewFakeApiResponder()

testutil.EnqueueRootResponder(rt)

rt.EnqueueResponder("GET", "/api/spaces/all", func(r *http.Request) (any, error) {
return []*spaces.Space{integrationsSpace}, nil
})

// we need to enqueue this again because after it finds Spaces-7 it will recreate the client and reload the root.
testutil.EnqueueRootResponder(rt)

// note it just goes for /api/Spaces-7 this time
rt.EnqueueResponder("GET", "/api/Spaces-7", space7responder)
rt.EnqueueResponder("GET", "/api/Spaces-7", func(r *http.Request) (any, error) {
return integrationsSpace, nil
})

factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", PlaceholderApiKey, "Spaces-7", nil)
factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", placeholderApiKey, "spaCeS-7", nil)

apiClient, err := factory2.GetSpacedClient()
assert.Nil(t, err)
Expand All @@ -116,15 +149,32 @@ func TestClient_GetSpacedClient_NoPrompt(t *testing.T) {
rt := testutil.NewFakeApiResponder()
testutil.EnqueueRootResponder(rt)

integrationsSpace := spaces.NewSpace("Integrations")
integrationsSpace.ID = "Spaces-7"
rt.EnqueueResponder("GET", "/api/spaces/all", func(r *http.Request) (any, error) {
return []*spaces.Space{integrationsSpace}, nil
})

// we need to enqueue this again because after it finds Spaces-7 it will recreate the client and reload the root.
testutil.EnqueueRootResponder(rt)

rt.EnqueueRawResponder("GET", "/api/spaces/Integrations", func(r *http.Request) (*http.Response, error) {
return &http.Response{StatusCode: 404}, nil
// note it just goes for /api/Spaces-7 this time
rt.EnqueueResponder("GET", "/api/Spaces-7", func(r *http.Request) (any, error) {
return integrationsSpace, nil
})

rt.EnqueueResponder("GET", "/api/spaces?partialName=Integrations", func(r *http.Request) (any, error) {
return resources.Resources[*spaces.Space]{Items: []*spaces.Space{integrationsSpace}}, nil
factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", placeholderApiKey, "Integrations", nil)

apiClient, err := factory2.GetSpacedClient()
assert.Nil(t, err)
assert.NotNil(t, apiClient)
assert.Equal(t, 0, rt.RemainingQueueLength())
})

t.Run("GetSpacedClient works when the Space Name is directly specified (case insensitive)", func(t *testing.T) {
rt := testutil.NewFakeApiResponder()
testutil.EnqueueRootResponder(rt)

rt.EnqueueResponder("GET", "/api/spaces/all", func(r *http.Request) (any, error) {
return []*spaces.Space{integrationsSpace}, nil
})

// we need to enqueue this again because after it finds Spaces-7 it will recreate the client and reload the root.
Expand All @@ -135,31 +185,59 @@ func TestClient_GetSpacedClient_NoPrompt(t *testing.T) {
return integrationsSpace, nil
})

factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", PlaceholderApiKey, "Integrations", nil)
factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", placeholderApiKey, "iNtegrationS", nil)

apiClient, err := factory2.GetSpacedClient()
assert.Nil(t, err)
assert.NotNil(t, apiClient)
assert.Equal(t, 0, rt.RemainingQueueLength())
})

t.Run("GetSpacedClient called twice returns the same client instance without additional requests", func(t *testing.T) {
t.Run("GetSpacedClient will select by name in preference to ID where there is a collision", func(t *testing.T) {
missedSpace := spaces.NewSpace("Missed")
missedSpace.ID = "Spaces-7"

spaces7space := spaces.NewSpace("Spaces-7") // nobody would do this in reality, but our software must still work properly
spaces7space.ID = "Spaces-209"

rt := testutil.NewFakeApiResponder()
testutil.EnqueueRootResponder(rt)

integrationsSpace := spaces.NewSpace("Integrations")
integrationsSpace.ID = "Spaces-7"
rt.EnqueueResponder("GET", "/api/spaces/all", func(r *http.Request) (any, error) {
return []*spaces.Space{
missedSpace,
spaces7space,
}, nil
})

testutil.EnqueueRootResponder(rt)

rt.EnqueueResponder("GET", "/api/spaces/Spaces-7", func(r *http.Request) (any, error) { return integrationsSpace, nil })
rt.EnqueueResponder("GET", "/api/Spaces-209", func(r *http.Request) (any, error) {
return spaces7space, nil
})

// we need to enqueue this again because after it finds Spaces-7 it will recreate the client and reload the root.
factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", placeholderApiKey, "Spaces-7", nil)

apiClient, err := factory2.GetSpacedClient()
assert.Nil(t, err)
assert.NotNil(t, apiClient)
assert.Equal(t, 0, rt.RemainingQueueLength())
})

t.Run("GetSpacedClient called twice returns the same client instance without additional requests", func(t *testing.T) {
rt := testutil.NewFakeApiResponder()
testutil.EnqueueRootResponder(rt)

// note it just goes for /api/Spaces-7 this time
rt.EnqueueResponder("GET", "/api/Spaces-7", func(r *http.Request) (any, error) { return integrationsSpace, nil })
rt.EnqueueResponder("GET", "/api/spaces/all", func(r *http.Request) (any, error) {
return []*spaces.Space{integrationsSpace}, nil
})

testutil.EnqueueRootResponder(rt)
rt.EnqueueResponder("GET", "/api/Spaces-7", func(r *http.Request) (any, error) {
return integrationsSpace, nil
})

factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", PlaceholderApiKey, "Spaces-7", nil)
factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", placeholderApiKey, "Spaces-7", nil)

apiClient, err := factory2.GetSpacedClient()
assert.Nil(t, err)
Expand All @@ -173,10 +251,10 @@ func TestClient_GetSpacedClient_NoPrompt(t *testing.T) {
}

func TestClient_GetSpacedClient_Prompt(t *testing.T) {
t.Run("GetSpacedClient auto-selects the first space when only one exists", func(t *testing.T) {
integrationsSpace := spaces.NewSpace("Integrations")
integrationsSpace.ID = "Spaces-23"
integrationsSpace := spaces.NewSpace("Integrations")
integrationsSpace.ID = "Spaces-23"

t.Run("GetSpacedClient auto-selects the first space when only one exists", func(t *testing.T) {
rt := testutil.NewFakeApiResponder()
testutil.EnqueueRootResponder(rt)

Expand All @@ -193,7 +271,7 @@ func TestClient_GetSpacedClient_Prompt(t *testing.T) {
asker, unasked := testutil.NewAskMocker(t, []testutil.QA{})
defer unasked()

factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", PlaceholderApiKey, "", asker)
factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", placeholderApiKey, "", asker)

apiClient, err := factory2.GetSpacedClient()
assert.Nil(t, err)
Expand All @@ -202,9 +280,6 @@ func TestClient_GetSpacedClient_Prompt(t *testing.T) {
})

t.Run("GetSpacedClient prompts for selection when more than one space exists", func(t *testing.T) {
integrationsSpace := spaces.NewSpace("Integrations")
integrationsSpace.ID = "Spaces-23"

cloudSpace := spaces.NewSpace("Cloud")
cloudSpace.ID = "Spaces-39"

Expand All @@ -229,7 +304,7 @@ func TestClient_GetSpacedClient_Prompt(t *testing.T) {
testutil.EnqueueRootResponder(rt)
rt.EnqueueResponder("GET", "/api/Spaces-39", func(r *http.Request) (any, error) { return integrationsSpace, nil })

factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", PlaceholderApiKey, "", asker)
factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", placeholderApiKey, "", asker)

apiClient, err := factory2.GetSpacedClient()
assert.Nil(t, err)
Expand All @@ -241,27 +316,23 @@ func TestClient_GetSpacedClient_Prompt(t *testing.T) {
rt := testutil.NewFakeApiResponder()
testutil.EnqueueRootResponder(rt)

// first it guesses that we might have a space ID
rt.EnqueueRawResponder("GET", "/api/spaces/Integrations", func(r *http.Request) (*http.Response, error) {
return &http.Response{StatusCode: 404}, nil
})
cloudSpace := spaces.NewSpace("CloudSpace")
cloudSpace.ID = "Spaces-39"

// then it tries a partial name search
rt.EnqueueResponder("GET", "/api/spaces?partialName=Integrations", func(r *http.Request) (any, error) {
return resources.Resources[*spaces.Space]{Items: []*spaces.Space{
spaces.NewSpace("NotIntegrations"),
}}, nil
rt.EnqueueResponder("GET", "/api/spaces/all", func(r *http.Request) (any, error) {
return []*spaces.Space{cloudSpace}, nil
})

// question/answer doesn't matter, just the presence of the mock signals it's allowed to auto-select the space
asker, unasked := testutil.NewAskMocker(t, []testutil.QA{})
defer unasked()

factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", PlaceholderApiKey, "Integrations", asker)
factory2, _ := apiclient.NewClientFactory(testutil.NewMockHttpClientWithTransport(rt), "http://server", placeholderApiKey, "Integrations", asker)

apiClient, err := factory2.GetSpacedClient()
assert.Nil(t, apiClient)
assert.Equal(t, "cannot use specified space 'Integrations'. Error: cannot find the item", err.Error()) // some strongly-typed errors would probably be nicer
assert.Equal(t, "cannot find space 'Integrations'", err.Error()) // some strongly-typed errors would probably be nicer
assert.Equal(t, 0, rt.RemainingQueueLength())
})
}
Loading

0 comments on commit 0d42bc0

Please sign in to comment.