From 718503c5ecb3b0a990e312fdfb41073cdcffc718 Mon Sep 17 00:00:00 2001
From: Jake Coffman <jakecoffman@github.com>
Date: Fri, 17 Nov 2023 12:51:31 -0600
Subject: [PATCH] fix flaky server input test (#198)

---
 .github/workflows/ci.yml              |  2 +-
 cmd/dependabot/internal/cmd/update.go |  7 ++++-
 internal/server/input.go              | 25 ++++++++---------
 internal/server/input_test.go         | 40 ++++++++++++++++++---------
 4 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 47c4e2d..6307a7f 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -22,7 +22,7 @@ jobs:
       - name: Test
         # -count=2 ensures that test fixtures cleanup after themselves
         # because any leftover state will generally cause the second run to fail.
-        run: go test -shuffle=on -count=2 -race -cover -v ./...
+        run: go test -shuffle=on -count=2 -race -cover -v -timeout=5m ./...
 
   lint:
     runs-on: ubuntu-latest
diff --git a/cmd/dependabot/internal/cmd/update.go b/cmd/dependabot/internal/cmd/update.go
index e5f3d3b..1ecc1ab 100644
--- a/cmd/dependabot/internal/cmd/update.go
+++ b/cmd/dependabot/internal/cmd/update.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"net"
 	"os"
 
 	"github.com/MakeNowJust/heredoc"
@@ -140,7 +141,11 @@ func extractInput(cmd *cobra.Command, flags *UpdateFlags) (*model.Input, error)
 	}
 
 	if hasServer {
-		return server.Input(flags.inputServerPort)
+		l, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", flags.inputServerPort))
+		if err != nil {
+			return nil, fmt.Errorf("failed to create listener: %w", err)
+		}
+		return server.Input(l)
 	}
 
 	if hasStdin {
diff --git a/internal/server/input.go b/internal/server/input.go
index 151766e..3e81c7c 100644
--- a/internal/server/input.go
+++ b/internal/server/input.go
@@ -3,12 +3,11 @@ package server
 import (
 	"context"
 	"encoding/json"
-	"fmt"
+	"errors"
+	"github.com/dependabot/cli/internal/model"
 	"log"
+	"net"
 	"net/http"
-	"time"
-
-	"github.com/dependabot/cli/internal/model"
 )
 
 type credServer struct {
@@ -29,17 +28,15 @@ func (s *credServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 }
 
 // Input receives configuration via HTTP on the port and returns it decoded
-func Input(port int) (*model.Input, error) {
-	server := &http.Server{
-		Addr:              fmt.Sprintf("127.0.0.1:%d", port),
-		ReadHeaderTimeout: time.Second,
-	}
-	s := &credServer{server: server}
-	server.Handler = s
+func Input(listener net.Listener) (*model.Input, error) {
+	handler := &credServer{}
+	srv := &http.Server{Handler: handler}
+	handler.server = srv
+
 	// printing so the user doesn't think the cli is hanging
-	log.Println("waiting for input on port", port)
-	if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+	log.Println("waiting for input on", listener.Addr())
+	if err := srv.Serve(listener); err != nil && !errors.Is(err, http.ErrServerClosed) {
 		return nil, err
 	}
-	return s.data, nil
+	return handler.data, nil
 }
diff --git a/internal/server/input_test.go b/internal/server/input_test.go
index c5981a3..f7cf27d 100644
--- a/internal/server/input_test.go
+++ b/internal/server/input_test.go
@@ -2,34 +2,48 @@ package server
 
 import (
 	"bytes"
+	"fmt"
+	"github.com/dependabot/cli/internal/model"
+	"net"
 	"net/http"
-	"sync"
+	"os"
 	"testing"
-	"time"
-
-	"github.com/dependabot/cli/internal/model"
 )
 
 func TestInput(t *testing.T) {
-	wg := sync.WaitGroup{}
-	wg.Add(1)
-	var input *model.Input
+	inputCh := make(chan *model.Input)
+	defer close(inputCh)
+
+	ip := ""
+	// prevents security popup
+	if os.Getenv("GOOS") == "darwin" {
+		ip = "127.0.0.1"
+	}
+	l, err := net.Listen("tcp", ip+":0")
+	if err != nil {
+		t.Fatal("Failed to create listener: ", err.Error())
+	}
+
 	go func() {
-		input, _ = Input(8080)
-		wg.Done()
+		input, err := Input(l)
+		if err != nil {
+			t.Errorf(err.Error())
+		}
+		inputCh <- input
 	}()
-	// give the server time to start
-	time.Sleep(10 * time.Millisecond)
 
+	url := fmt.Sprintf("http://%s", l.Addr().String())
 	data := `{"job":{"package-manager":"test"},"credentials":[{"credential":"value"}]}`
-	resp, err := http.Post("http://localhost:8080", "application/json", bytes.NewReader([]byte(data)))
+	resp, err := http.Post(url, "application/json", bytes.NewReader([]byte(data)))
 	if err != nil {
 		t.Fatal(err.Error())
 	}
 	if resp.StatusCode != 200 {
 		t.Errorf("expected status code 200, got %d", resp.StatusCode)
 	}
-	wg.Wait()
+
+	// Test will hang here if the server does not shut down
+	input := <-inputCh
 
 	if input.Job.PackageManager != "test" {
 		t.Errorf("expected package manager to be 'test', got '%s'", input.Job.PackageManager)