Skip to content

Commit

Permalink
Attempt to fix flaky TestGetRoundTripper* tests (#4738)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- Resolves #4732

## Description of the changes
- Introduces an atomic boolean outside of the HTTP test handler and only
set to `true` if the expected request is received.
- This should prevent cases where unrelated processes are hitting the
same test URL.

## How was this change tested?
- Ran `make test` to pass.
- Ran `go test -tags=memory_storage_integration -count 10 ./...` and
confirmed `metricsstore` tests are passing.
- Note that a number of unrelated tests were failing with the `-count
10` flag set, **not as a result of the changes from this PR**, namely:
```
--- FAIL: TestFactory (0.00s)
    --- FAIL: TestFactory/#00 (0.00s)
panic: Reuse of exported var name: test_1694287567920905000_counter_x
 [recovered]
	panic: Reuse of exported var name: test_1694287567920905000_counter_x
```
and
```
--- FAIL: TestPublishOpts (0.00s)
    builder_test.go:308:
        	Error Trace:	/Users/albertteoh/go/src/github.com/albertteoh/jaeger/cmd/agent/app/builder_test.go:308
        	Error:      	Received unexpected error:
        	            	cannot create processors: cannot create Thrift Processor: cannot create UDP Server: cannot create UDPServerTransport: listen udp :5775: bind: address already in use
        	Test:       	TestPublishOpts
```

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Albert Teoh <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Co-authored-by: Albert Teoh <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
  • Loading branch information
3 people authored Sep 9, 2023
1 parent 43a54dc commit 81d2cf8
Showing 1 changed file with 43 additions and 28 deletions.
71 changes: 43 additions & 28 deletions plugin/metrics/prometheus/metricsstore/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ package metricsstore

import (
"context"
"fmt"
"io"
"net"
"net/http"
"net/http/httptest"
"net/url"
"os"
"strings"
"sync/atomic"
"testing"
"time"

Expand Down Expand Up @@ -518,6 +518,29 @@ func TestWarningResponse(t *testing.T) {
assert.Len(t, exp.GetSpans(), 1, "HTTP request was traced and span reported")
}

type fakePromServer struct {
*httptest.Server
authReceived atomic.Pointer[string]
}

func newFakePromServer(t *testing.T) *fakePromServer {
s := &fakePromServer{}
s.Server = httptest.NewServer(
http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
t.Logf("Request to fake Prometheus server %+v", r)
h := r.Header.Get("Authorization")
s.authReceived.Store(&h)
},
),
)
return s
}

func (s *fakePromServer) getAuth() string {
return *s.authReceived.Load()
}

func TestGetRoundTripperTLSConfig(t *testing.T) {
for _, tc := range []struct {
name string
Expand All @@ -537,13 +560,7 @@ func TestGetRoundTripperTLSConfig(t *testing.T) {
}, logger)
require.NoError(t, err)

server := httptest.NewServer(
http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "Bearer foo", r.Header.Get("Authorization"))
},
),
)
server := newFakePromServer(t)
defer server.Close()

req, err := http.NewRequestWithContext(
Expand All @@ -556,7 +573,9 @@ func TestGetRoundTripperTLSConfig(t *testing.T) {

resp, err := rt.RoundTrip(req)
require.NoError(t, err)

assert.Equal(t, http.StatusOK, resp.StatusCode)
assert.Equal(t, "Bearer foo", server.getAuth())
})
}
}
Expand All @@ -578,14 +597,10 @@ func TestGetRoundTripperTokenFile(t *testing.T) {
TokenOverrideFromContext: false,
}, nil)
require.NoError(t, err)
server := httptest.NewServer(
http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "Bearer "+wantBearer, r.Header.Get("Authorization"))
},
),
)

server := newFakePromServer(t)
defer server.Close()

ctx := bearertoken.ContextWithBearerToken(context.Background(), "tokenFromRequest")
req, err := http.NewRequestWithContext(
ctx,
Expand All @@ -594,9 +609,12 @@ func TestGetRoundTripperTokenFile(t *testing.T) {
nil,
)
require.NoError(t, err)

resp, err := rt.RoundTrip(req)
require.NoError(t, err)

assert.Equal(t, http.StatusOK, resp.StatusCode)
assert.Equal(t, "Bearer "+wantBearer, server.getAuth())
}

func TestGetRoundTripperTokenFromContext(t *testing.T) {
Expand All @@ -613,14 +631,10 @@ func TestGetRoundTripperTokenFromContext(t *testing.T) {
TokenOverrideFromContext: true,
}, nil)
require.NoError(t, err)
server := httptest.NewServer(
http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "Bearer tokenFromRequest", r.Header.Get("Authorization"))
},
),
)

server := newFakePromServer(t)
defer server.Close()

ctx := bearertoken.ContextWithBearerToken(context.Background(), "tokenFromRequest")
req, err := http.NewRequestWithContext(
ctx,
Expand All @@ -629,9 +643,12 @@ func TestGetRoundTripperTokenFromContext(t *testing.T) {
nil,
)
require.NoError(t, err)

resp, err := rt.RoundTrip(req)
require.NoError(t, err)

assert.Equal(t, http.StatusOK, resp.StatusCode)
assert.Equal(t, "Bearer tokenFromRequest", server.getAuth())
}

func TestGetRoundTripperTokenError(t *testing.T) {
Expand Down Expand Up @@ -685,14 +702,12 @@ func startMockPrometheusServer(t *testing.T, wantPromQlQuery string, wantWarning
}))
}

func sendResponse(t *testing.T, w http.ResponseWriter, responseFile string) error {
func sendResponse(t *testing.T, w http.ResponseWriter, responseFile string) {
bytes, err := os.ReadFile(responseFile)
if err != nil {
return err
}
require.NoError(t, err)

_, err = fmt.Fprintln(w, string(bytes))
return err
_, err = w.Write(bytes)
require.NoError(t, err)
}

func buildTestBaseQueryParametersFrom(tc metricsTestCase) metricsstore.BaseQueryParameters {
Expand Down

0 comments on commit 81d2cf8

Please sign in to comment.