skipper: fix shutdown test

* verify request is still ongoing after shutdown started
* verify shutdown completes after request is finished
* rename tests to have the ame prefix

Updates #988 #1865

Signed-off-by: Alexander Yastrebov <[email protected]>

skipper_test.go

@@ -283,19 +283,24 @@ func TestHTTPServer(t *testing.T) {
 	}
 }
 
-func TestHTTPServerShutdown(t *testing.T) {
+func TestServerShutdownHTTP(t *testing.T) {
 	o := &Options{}
 	testServerShutdown(t, o, "http")
 }
 
-func TestHTTPSServerShutdown(t *testing.T) {
+func TestServerShutdownHTTPS(t *testing.T) {
 	o := &Options{
 		CertPathTLS: "fixtures/test.crt",
 		KeyPathTLS:  "fixtures/test.key",
 	}
 	testServerShutdown(t, o, "https")
 }
 
+type responseOrError struct {
+	rsp *http.Response
+	err error
+}
+
 func testServerShutdown(t *testing.T, o *Options, scheme string) {
 	const shutdownDelay = 1 * time.Second
 
@@ -319,8 +324,9 @@ func testServerShutdown(t *testing.T, o *Options, scheme string) {
 	defer proxy.Close()
 
 	sigs := make(chan os.Signal, 1)
+	done := make(chan struct{})
 	go func() {
-		err := listenAndServeQuit(proxy, o, sigs, nil, nil, nil)
+		err := listenAndServeQuit(proxy, o, sigs, done, nil, nil)
 		require.NoError(t, err)
 	}()
 
@@ -329,22 +335,39 @@ func testServerShutdown(t *testing.T, o *Options, scheme string) {
 
 	time.Sleep(shutdownDelay / 2)
 
-	t.Logf("ongoing request passing in before shutdown")
-	r, err := waitConnGet(testUrl)
-	require.NoError(t, err)
-	require.Equal(t, 200, r.StatusCode)
+	t.Logf("Make request in parallel before shutdown started")
 
-	defer r.Body.Close()
+	roeCh := make(chan responseOrError)
+	go func() {
+		rsp, err := waitConnGet(testUrl)
+		roeCh <- responseOrError{rsp, err}
+	}()
 
-	body, err := io.ReadAll(r.Body)
-	require.NoError(t, err)
-	require.Equal(t, "OK", string(body))
+	time.Sleep(shutdownDelay)
 
-	time.Sleep(shutdownDelay / 2)
+	t.Logf("We are 1.5x past the shutdown delay, so the server should be shutting down now")
 
-	t.Logf("request after shutdown should fail")
-	_, err = waitConnGet(testUrl)
-	require.Error(t, err)
+	select {
+	case <-roeCh:
+		t.Fatalf("Request should still be in progress after shutdown started")
+	default:
+		_, err = waitConnGet(testUrl)
+		assert.ErrorContains(t, err, "connection refused", "Another request should fail after shutdown started")
+	}
+
+	roe := <-roeCh
+	require.NoError(t, roe.err, "Request started before shutdown must succeed")
+	defer roe.rsp.Body.Close()
+
+	body, err := io.ReadAll(roe.rsp.Body)
+	require.NoError(t, err)
+	assert.Equal(t, "OK", string(body))
+
+	select {
+	case <-done:
+	case <-time.After(1 * time.Second):
+		t.Errorf("Shutdown takes too long after request is finished")
+	}
 }
 
 type (