From 9f78c2834094c90407f26ddce43e6a296140afa6 Mon Sep 17 00:00:00 2001
From: James Peach <jpeach@vmware.com>
Date: Mon, 20 Jul 2020 13:52:01 +1000
Subject: [PATCH] Fix test comparison stability. (#6)

Internally, the check response heaers use maps, so the header slice
ordering is not stable. Add a test helper to sort this for stable test
comparisons.

Signed-off-by: James Peach <jpeach@vmware.com>
Signed-off-by: robinfoe <robin@rfoe-a01.vmware.com>
---
 pkg/auth/convert_test.go | 57 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 7 deletions(-)

diff --git a/pkg/auth/convert_test.go b/pkg/auth/convert_test.go
index f3ffafd..0c2f4fc 100644
--- a/pkg/auth/convert_test.go
+++ b/pkg/auth/convert_test.go
@@ -16,6 +16,7 @@ package auth
 import (
 	"net/http"
 	"net/url"
+	"sort"
 	"testing"
 
 	envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
@@ -55,6 +56,40 @@ func testConvertedRequest(t *testing.T, actual *Request) {
 	assert.True(t, ok, "expected.BasicAuth() should return true")
 }
 
+// stableV2 sorts the headers slice so that test equality is deterministic.
+func stableV2(c *CheckResponseV2) *CheckResponseV2 {
+	if d := c.GetDeniedResponse(); d != nil {
+		sort.Slice(d.Headers, func(i, j int) bool {
+			return d.Headers[i].GetHeader().GetKey() < d.Headers[j].GetHeader().GetKey()
+		})
+	}
+
+	if o := c.GetOkResponse(); o != nil {
+		sort.Slice(o.Headers, func(i, j int) bool {
+			return o.Headers[i].GetHeader().GetKey() < o.Headers[j].GetHeader().GetKey()
+		})
+	}
+
+	return c
+}
+
+// stableV3 sorts the headers slice so that test equality is deterministic.
+func stableV3(c *CheckResponseV3) *CheckResponseV3 {
+	if d := c.GetDeniedResponse(); d != nil {
+		sort.Slice(d.Headers, func(i, j int) bool {
+			return d.Headers[i].GetHeader().GetKey() < d.Headers[j].GetHeader().GetKey()
+		})
+	}
+
+	if o := c.GetOkResponse(); o != nil {
+		sort.Slice(o.Headers, func(i, j int) bool {
+			return o.Headers[i].GetHeader().GetKey() < o.Headers[j].GetHeader().GetKey()
+		})
+	}
+
+	return c
+}
+
 func TestConvertRequestV2(t *testing.T) {
 	in := CheckRequestV2{
 		Attributes: &envoy_service_auth_v2.AttributeContext{
@@ -129,7 +164,7 @@ func TestConvertDenied(t *testing.T) {
 		},
 	}
 
-	assert.Equal(t, response.AsV2(),
+	assert.Equal(t, stableV2(response.AsV2()),
 		&CheckResponseV2{
 			Status: &status.Status{
 				Code: int32(codes.PermissionDenied),
@@ -156,7 +191,7 @@ func TestConvertDenied(t *testing.T) {
 		},
 	)
 
-	assert.Equal(t, response.AsV3(),
+	assert.Equal(t, stableV3(response.AsV3()),
 		&CheckResponseV3{
 			Status: &status.Status{
 				Code: int32(codes.PermissionDenied),
@@ -190,15 +225,13 @@ func TestConvertAllowed(t *testing.T) {
 		Response: http.Response{
 			StatusCode: 415,
 			Header: http.Header{
-				// We only have 1 header here, so that we don't run into spurious
-				// test failures by emitting the Envoy headers slice in an undefined
-				// order.
 				"k1": {"v1"},
+				"k2": {"v2"},
 			},
 		},
 	}
 
-	assert.Equal(t, response.AsV2(),
+	assert.Equal(t, stableV2(response.AsV2()),
 		&CheckResponseV2{
 			Status: &status.Status{
 				Code: int32(codes.OK),
@@ -211,13 +244,18 @@ func TestConvertAllowed(t *testing.T) {
 								Key: "k1", Value: "v1",
 							},
 						},
+						{
+							Header: &envoy_api_v2_core.HeaderValue{
+								Key: "k2", Value: "v2",
+							},
+						},
 					},
 				},
 			},
 		},
 	)
 
-	assert.Equal(t, response.AsV3(),
+	assert.Equal(t, stableV3(response.AsV3()),
 		&CheckResponseV3{
 			Status: &status.Status{
 				Code: int32(codes.OK),
@@ -230,6 +268,11 @@ func TestConvertAllowed(t *testing.T) {
 								Key: "k1", Value: "v1",
 							},
 						},
+						{
+							Header: &envoy_config_core_v3.HeaderValue{
+								Key: "k2", Value: "v2",
+							},
+						},
 					},
 				},
 			},