chore: Fix small error and warnings in tests, use dockerhub for octop…

…us server image rather than packages.octopushq (#333)

* Cleanup small warnings and errors in CLI tests

* Run integration tests against latest from dockerhub, rather than artifactory

.github/workflows/integration-test.yml

@@ -31,7 +31,7 @@ jobs:
           --health-retries 10
           --health-start-period 10s
       octopusserver:
-        image: docker.packages.octopushq.com/octopusdeploy/octopusdeploy:2023.4.8126-linux
+        image: octopusdeploy/octopusdeploy:latest
         env:
           ACCEPT_EULA: Y
           DB_CONNECTION_STRING: "Server=sqlserver;Database=OctopusDeploy;User Id=sa;Password=${{ env.SA_PASSWORD }};"

pkg/apiclient/client_factory.go

@@ -391,11 +391,11 @@ func NewStubClientFactory() ClientFactory {
 
 type stubClientFactory struct{}
 
-func (s *stubClientFactory) GetSpacedClient(requester Requester) (*octopusApiClient.Client, error) {
+func (s *stubClientFactory) GetSpacedClient(_ Requester) (*octopusApiClient.Client, error) {
 	return nil, errors.New("app is not configured correctly")
 }
 
-func (s *stubClientFactory) GetSystemClient(requester Requester) (*octopusApiClient.Client, error) {
+func (s *stubClientFactory) GetSystemClient(_ Requester) (*octopusApiClient.Client, error) {
 	return nil, errors.New("app is not configured correctly")
 }
 

pkg/apiclient/requester.go

@@ -28,22 +28,22 @@ func NewRequester(c *cobra.Command) *RequesterContext {
 func (r *FakeRequesterContext) GetRequester() string { return "octopus/0.0.0" }
 
 func (r *RequesterContext) GetRequester() string {
-	version := strings.TrimSpace(version.Version)
+	versionStr := strings.TrimSpace(version.Version)
 
 	if r.cmd == nil {
-		if version == "" {
+		if versionStr == "" {
 			return constants.ExecutableName
 		}
-		return fmt.Sprintf("%s/%s", constants.ExecutableName, version)
+		return fmt.Sprintf("%s/%s", constants.ExecutableName, versionStr)
 	}
 
 	commands := []string{r.cmd.Name()}
 	var rootCmd string
 	parentCmd := r.cmd.Parent()
 	for parentCmd != nil {
 		name := parentCmd.Name()
-		if name == constants.ExecutableName && version != "" {
-			rootCmd = fmt.Sprintf("%s/%s", name, version)
+		if name == constants.ExecutableName && versionStr != "" {
+			rootCmd = fmt.Sprintf("%s/%s", name, versionStr)
 		} else {
 			commands = append([]string{name}, commands...)
 		}

test/testutil/fakefactory.go

@@ -71,7 +71,7 @@ type MockFactory struct {
 const serverUrl = "http://server"
 const placeholderApiKey = "API-XXXXXXXXXXXXXXXXXXXXXXXXXXXXX"
 
-func (f *MockFactory) GetSystemClient(requester apiclient.Requester) (*octopusApiClient.Client, error) {
+func (f *MockFactory) GetSystemClient(_ apiclient.Requester) (*octopusApiClient.Client, error) {
 	serverUrl, _ := url.Parse(serverUrl)
 
 	if f.SystemClient == nil {
@@ -83,7 +83,7 @@ func (f *MockFactory) GetSystemClient(requester apiclient.Requester) (*octopusAp
 	}
 	return f.SystemClient, nil
 }
-func (f *MockFactory) GetSpacedClient(requester apiclient.Requester) (*octopusApiClient.Client, error) {
+func (f *MockFactory) GetSpacedClient(_ apiclient.Requester) (*octopusApiClient.Client, error) {
 	if f.CurrentSpace == nil {
 		return nil, errors.New("can't get space-scoped client from MockFactory while CurrentSpace is nil")
 	}

test/testutil/fakeoctopusserver.go

@@ -4,7 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"sync/atomic"
 	"testing"
@@ -73,15 +73,15 @@ type MockHttpServer struct {
 	Response chan responseOrError
 
 	// so test code can detect unanswered requests or responses at the end.
-	// Not strictly neccessary as unanswered req/resp results in a channel deadlock
+	// Not strictly necessary as unanswered req/resp results in a channel deadlock
 	// and go panics and kills the process, so we find out about it, but this is a bit
 	// less confusing to troubleshoot
 	pendingMsgCount int32
 
 	Closed bool
 }
 
-// conforms to RoundTripper so we can use it for httpClient.Transport
+// conforms to RoundTripper, so we can use it for httpClient.Transport
 
 func (m *MockHttpServer) RoundTrip(r *http.Request) (*http.Response, error) {
 	// we're the client here, so we send a request down the request channel
@@ -135,7 +135,7 @@ func (m *MockHttpServer) Respond(response *http.Response, err error) {
 func (m *MockHttpServer) ExpectRequest(t *testing.T, method string, pathAndQuery string) *RequestWrapper {
 	r, ok := m.ReceiveRequest()
 	if !ok { // this means the channel was closed
-		// don't fatal, there'll be some other assertion failure too and we want to let that have a chance to print
+		// don't fatal, there'll be some other assertion failure too, and we want to let that have a chance to print
 		t.Errorf("ExpectRequest %s %s failed; channel closed", method, pathAndQuery)
 		return &RequestWrapper{&http.Request{}, m}
 	}
@@ -190,7 +190,7 @@ func (r *RequestWrapper) RespondWithStatus(statusCode int, statusString string,
 	r.Server.Respond(&http.Response{
 		StatusCode:    statusCode,
 		Status:        statusString,
-		Body:          ioutil.NopCloser(bytes.NewReader(body)),
+		Body:          io.NopCloser(bytes.NewReader(body)),
 		ContentLength: int64(len(body)),
 	}, nil)
 }

test/testutil/fakesurvey.go

@@ -4,7 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"github.com/OctopusDeploy/cli/pkg/surveyext"
-	"slices"
+	"golang.org/x/exp/slices"
 	"testing"
 
 	"github.com/AlecAivazis/survey/v2"
@@ -241,7 +241,7 @@ func (m *AskMocker) AsAsker() func(p survey.Prompt, response interface{}, opts .
 		// then we wait for a response via the answer channel.
 		// NOTE validations should have already been run on the send side, so we should only receive things
 		// that have passed any survey validators. We mostly do this because the concurrent nature of this
-		// makes it much easier to have the "AnswerWith" do the validation than the more correct place (here.
+		// makes it much easier to have the "AnswerWith" do the validation than the more correct place here.
 		x := <-m.Answer
 
 		if x.answer != nil {

test/testutil/testutil.go

@@ -62,6 +62,7 @@ func NewMockHttpClientWithTransport(transport http.RoundTripper) *http.Client {
 	return httpClient
 }
 
+// ReadJson reads from `body`, then calls json.Unmarshal to try load JSON into object of type T
 // NOTE max length of 8k
 func ReadJson[T any](body io.ReadCloser) (T, error) {
 	if body == nil {
@@ -83,13 +84,15 @@ func ReadJson[T any](body io.ReadCloser) (T, error) {
 	return unmarshalled, nil
 }
 
+// NewMockServerAndAsker creates both a MockHttpServer and AskMocker
 // it's super common to New both the mock server and asker at the same time
 func NewMockServerAndAsker() (*MockHttpServer, *AskMocker) {
 	server := NewMockHttpServer()
 	qa := NewAskMocker()
 	return server, qa
 }
 
+// Close will close both a MockHttpServer and AskMocker
 // it's super common to Close both the mock server and asker at the same time
 func Close(server *MockHttpServer, qa *AskMocker) {
 	if server != nil {
@@ -129,10 +132,13 @@ func CaptureConsoleOutput(f func()) string {
 	}()
 
 	f()
-	w.Close()
+	_ = w.Close()
 
 	var buf bytes.Buffer
-	io.Copy(&buf, r)
+	_, err = io.Copy(&buf, r)
+	if err != nil {
+		panic(err)
+	}
 
 	return buf.String()
 }