Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --trace option to print full trace of HTTP requests and responses for failed test cases #761

Merged
merged 4 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cmd/connectconformance/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const (
tlsKeyFlagName = "key"
portFlagName = "port"
bindFlagName = "bind"
traceFlagName = "trace"
)

type flags struct {
Expand All @@ -65,6 +66,7 @@ type flags struct {
tlsKeyFile string
port uint
bind string
trace bool
}

func main() {
Expand Down Expand Up @@ -157,6 +159,8 @@ func bind(cmd *cobra.Command, flags *flags) {
"in client mode, the port number on which the reference server should listen (implies --max-servers=1)")
cmd.Flags().StringVar(&flags.bind, bindFlagName, internal.DefaultHost,
"in client mode, the bind address on which the reference server should listen (0.0.0.0 means listen on all interfaces)")
cmd.Flags().BoolVar(&flags.trace, traceFlagName, false,
"if true, full HTTP traces will be captured and shown alongside failing test cases")
}

func run(flags *flags, cobraFlags *pflag.FlagSet, command []string) { //nolint:gocyclo
Expand Down Expand Up @@ -298,6 +302,7 @@ func run(flags *flags, cobraFlags *pflag.FlagSet, command []string) { //nolint:g
TLSKeyFile: flags.tlsKeyFile,
ServerPort: flags.port,
ServerBind: flags.bind,
HTTPTrace: flags.trace,
},
internal.NewPrinter(os.Stdout),
internal.NewPrinter(os.Stderr),
Expand Down
2 changes: 1 addition & 1 deletion cmd/referenceclient/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
)

func main() {
err := referenceclient.Run(context.Background(), os.Args, os.Stdin, os.Stdout, os.Stderr)
err := referenceclient.Run(context.Background(), os.Args, os.Stdin, os.Stdout, os.Stderr, nil)
if err != nil {
log.Fatalf("an error occurred running the reference client: %s", err.Error())
}
Expand Down
19 changes: 16 additions & 3 deletions internal/app/connectconformance/connectconformance.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"errors"
"fmt"
"io"
"os"
"path"
"sort"
Expand All @@ -32,6 +33,7 @@ import (
"connectrpc.com/conformance/internal/app/referenceclient"
"connectrpc.com/conformance/internal/app/referenceserver"
conformancev1 "connectrpc.com/conformance/internal/gen/proto/go/connectrpc/conformance/v1"
"connectrpc.com/conformance/internal/tracer"
"golang.org/x/sync/semaphore"
"google.golang.org/protobuf/proto"
)
Expand All @@ -54,6 +56,7 @@ type Flags struct {
TLSKeyFile string
ServerPort uint
ServerBind string
HTTPTrace bool
}

func Run(flags *Flags, logPrinter internal.Printer, errPrinter internal.Printer) (bool, error) {
Expand Down Expand Up @@ -221,6 +224,11 @@ func run( //nolint:gocyclo
}
}

var trace *tracer.Tracer
if flags.HTTPTrace {
trace = &tracer.Tracer{}
}

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

Expand All @@ -232,7 +240,9 @@ func run( //nolint:gocyclo
start: runInProcess([]string{
"reference-client",
"-p", strconv.Itoa(int(flags.Parallelism)),
}, referenceclient.Run),
}, func(ctx context.Context, args []string, inReader io.ReadCloser, outWriter, errWriter io.WriteCloser) error {
return referenceclient.Run(ctx, args, inReader, outWriter, errWriter, trace)
}),
isReferenceImpl: true,
},
{
Expand All @@ -252,7 +262,7 @@ func run( //nolint:gocyclo
}
}

results := newResults(knownFailing, knownFlaky)
results := newResults(knownFailing, knownFlaky, trace)

for _, clientInfo := range clients {
clientProcess, err := runClient(ctx, clientInfo.start)
Expand All @@ -272,7 +282,9 @@ func run( //nolint:gocyclo
"-bind", flags.ServerBind,
"-cert", flags.TLSCertFile,
"-key", flags.TLSKeyFile,
}, referenceserver.RunInReferenceMode),
}, func(ctx context.Context, args []string, inReader io.ReadCloser, outWriter, errWriter io.WriteCloser) error {
return referenceserver.RunInReferenceMode(ctx, args, inReader, outWriter, errWriter, trace)
}),
isReferenceImpl: true,
},
{
Expand Down Expand Up @@ -349,6 +361,7 @@ func run( //nolint:gocyclo
errPrinter,
results,
clientProcess,
trace,
)
}(ctx, clientInfo, serverInfo, svrInstance)
}
Expand Down
47 changes: 46 additions & 1 deletion internal/app/connectconformance/results.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,19 @@ package connectconformance

import (
"bytes"
"context"
"errors"
"fmt"
"reflect"
"sort"
"strconv"
"strings"
"sync"
"time"

"connectrpc.com/conformance/internal"
conformancev1 "connectrpc.com/conformance/internal/gen/proto/go/connectrpc/conformance/v1"
"connectrpc.com/conformance/internal/tracer"
"connectrpc.com/connect"
"github.com/google/go-cmp/cmp"
"google.golang.org/protobuf/proto"
Expand All @@ -42,16 +45,21 @@ const timeoutCheckGracePeriodMillis = 500
type testResults struct {
knownFailing *testTrie
knownFlaky *testTrie
tracer *tracer.Tracer

traceWaitGroup sync.WaitGroup

mu sync.Mutex
outcomes map[string]testOutcome
traces map[string]*tracer.Trace
serverSideband map[string]string
}

func newResults(knownFailing, knownFlaky *testTrie) *testResults {
func newResults(knownFailing, knownFlaky *testTrie, tracer *tracer.Tracer) *testResults {
return &testResults{
knownFailing: knownFailing,
knownFlaky: knownFlaky,
tracer: tracer,
outcomes: map[string]testOutcome{},
serverSideband: map[string]string{},
}
Expand All @@ -74,6 +82,36 @@ func (r *testResults) setOutcomeLocked(testCase string, setupError bool, err err
knownFailing: r.knownFailing.match(strings.Split(testCase, "/")),
knownFlaky: r.knownFlaky.match(strings.Split(testCase, "/")),
}
r.fetchTrace(testCase)
}

//nolint:contextcheck,nolintlint // intentionally using context.Background; nolintlint incorrectly complains about this
func (r *testResults) fetchTrace(testCase string) {
if r.tracer == nil {
return
}
r.traceWaitGroup.Add(1)
go func() {
defer r.traceWaitGroup.Done()
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
trace, err := r.tracer.Await(ctx, testCase)
r.tracer.Clear(testCase)
if err != nil {
return
}

r.mu.Lock()
defer r.mu.Unlock()
outcome := r.outcomes[testCase]
if outcome.actualFailure == nil || outcome.setupError || outcome.knownFlaky || outcome.knownFailing {
return
}
if r.traces == nil {
r.traces = map[string]*tracer.Trace{}
}
r.traces[testCase] = trace
}()
}

// failedToStart marks all the given test cases with the given setup error.
Expand Down Expand Up @@ -186,6 +224,7 @@ func (r *testResults) processSidebandInfoLocked() {
}

func (r *testResults) report(printer internal.Printer) bool {
r.traceWaitGroup.Wait() // make sure all traces have been received
r.mu.Lock()
defer r.mu.Unlock()
if len(r.serverSideband) > 0 {
Expand All @@ -208,6 +247,12 @@ func (r *testResults) report(printer internal.Printer) bool {
switch {
case !expectError && outcome.actualFailure != nil:
printer.Printf("FAILED: %s:\n%s", name, indent(outcome.actualFailure.Error()))
trace := r.traces[name]
if trace != nil {
printer.Printf("---- HTTP Trace ----")
trace.Print(printer)
printer.Printf("--------------------")
}
failed++
case expectError && outcome.actualFailure == nil:
printer.Printf("FAILED: %s was expected to fail but did not", name)
Expand Down
30 changes: 15 additions & 15 deletions internal/app/connectconformance/results_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (

func TestResults_SetOutcome(t *testing.T) {
t.Parallel()
results := newResults(makeKnownFailing(), makeKnownFlaky())
results := newResults(makeKnownFailing(), makeKnownFlaky(), nil)
results.setOutcome("foo/bar/1", false, nil)
results.setOutcome("foo/bar/2", true, errors.New("fail"))
results.setOutcome("foo/bar/3", false, errors.New("fail"))
Expand Down Expand Up @@ -58,7 +58,7 @@ func TestResults_SetOutcome(t *testing.T) {

func TestResults_FailedToStart(t *testing.T) {
t.Parallel()
results := newResults(makeKnownFailing(), makeKnownFlaky())
results := newResults(makeKnownFailing(), makeKnownFlaky(), nil)
results.failedToStart([]*conformancev1.TestCase{
{Request: &conformancev1.ClientCompatRequest{TestName: "foo/bar/1"}},
{Request: &conformancev1.ClientCompatRequest{TestName: "known-to-fail/1"}},
Expand All @@ -76,7 +76,7 @@ func TestResults_FailedToStart(t *testing.T) {

func TestResults_FailRemaining(t *testing.T) {
t.Parallel()
results := newResults(makeKnownFailing(), makeKnownFlaky())
results := newResults(makeKnownFailing(), makeKnownFlaky(), nil)
results.setOutcome("foo/bar/1", false, nil)
results.setOutcome("known-to-fail/1", false, errors.New("fail"))
results.failRemaining([]*conformancev1.TestCase{
Expand All @@ -101,7 +101,7 @@ func TestResults_FailRemaining(t *testing.T) {

func TestResults_Failed(t *testing.T) {
t.Parallel()
results := newResults(makeKnownFailing(), makeKnownFlaky())
results := newResults(makeKnownFailing(), makeKnownFlaky(), nil)
results.failed("foo/bar/1", &conformancev1.ClientErrorResult{Message: "fail"})
results.failed("known-to-fail/1", &conformancev1.ClientErrorResult{Message: "fail"})

Expand All @@ -116,7 +116,7 @@ func TestResults_Failed(t *testing.T) {

func TestResults_Assert(t *testing.T) {
t.Parallel()
results := newResults(makeKnownFailing(), makeKnownFlaky())
results := newResults(makeKnownFailing(), makeKnownFlaky(), nil)
payload1 := &conformancev1.ClientResponseResult{
Payloads: []*conformancev1.ConformancePayload{
{Data: []byte{0, 1, 2, 3, 4}},
Expand Down Expand Up @@ -688,7 +688,7 @@ func TestResults_Assert_ReportsAllErrors(t *testing.T) {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
results := newResults(&testTrie{}, &testTrie{})
results := newResults(&testTrie{}, &testTrie{}, nil)

expected := &conformancev1.ClientResponseResult{}
err := protojson.Unmarshal(([]byte)(testCase.expected), expected)
Expand Down Expand Up @@ -722,7 +722,7 @@ func TestResults_Assert_ReportsAllErrors(t *testing.T) {

func TestResults_ServerSideband(t *testing.T) {
t.Parallel()
results := newResults(makeKnownFailing(), makeKnownFlaky())
results := newResults(makeKnownFailing(), makeKnownFlaky(), nil)
results.setOutcome("foo/bar/1", false, nil)
results.setOutcome("foo/bar/2", false, errors.New("fail"))
results.setOutcome("foo/bar/3", false, nil)
Expand All @@ -745,50 +745,50 @@ func TestResults_ServerSideband(t *testing.T) {

func TestResults_Report(t *testing.T) {
t.Parallel()
results := newResults(makeKnownFailing(), makeKnownFlaky())
results := newResults(makeKnownFailing(), makeKnownFlaky(), nil)
logger := &linePrinter{}

// No test cases? Report success.
success := results.report(logger)
require.True(t, success)

// Only successful outcomes? Report success.
results = newResults(makeKnownFailing(), makeKnownFlaky())
results = newResults(makeKnownFailing(), makeKnownFlaky(), nil)
results.setOutcome("foo/bar/1", false, nil)
success = results.report(logger)
require.True(t, success)

// Unexpected failure? Report failure.
results = newResults(makeKnownFailing(), makeKnownFlaky())
results = newResults(makeKnownFailing(), makeKnownFlaky(), nil)
results.setOutcome("foo/bar/1", false, errors.New("ruh roh"))
success = results.report(logger)
require.False(t, success)

// Unexpected failure during setup? Report failure.
results = newResults(makeKnownFailing(), makeKnownFlaky())
results = newResults(makeKnownFailing(), makeKnownFlaky(), nil)
results.setOutcome("foo/bar/1", true, errors.New("ruh roh"))
success = results.report(logger)
require.False(t, success)

// Expected failure? Report success.
results = newResults(makeKnownFailing(), makeKnownFlaky())
results = newResults(makeKnownFailing(), makeKnownFlaky(), nil)
results.setOutcome("known-to-fail/1", false, errors.New("ruh roh"))
success = results.report(logger)
require.True(t, success)

// Setup error from expected failure? Report failure (setup errors never acceptable).
results = newResults(makeKnownFailing(), makeKnownFlaky())
results = newResults(makeKnownFailing(), makeKnownFlaky(), nil)
results.setOutcome("known-to-fail/1", true, errors.New("ruh roh"))
success = results.report(logger)
require.False(t, success)

// Flaky? Report success whether it passes or fails
results = newResults(makeKnownFailing(), makeKnownFlaky())
results = newResults(makeKnownFailing(), makeKnownFlaky(), nil)
results.setOutcome("known-to-flake/1", false, nil) // succeeds
success = results.report(logger)
require.True(t, success)

results = newResults(makeKnownFailing(), makeKnownFlaky())
results = newResults(makeKnownFailing(), makeKnownFlaky(), nil)
results.setOutcome("known-to-flake/1", false, errors.New("ruh roh"))
success = results.report(logger)
require.True(t, success)
Expand Down
3 changes: 3 additions & 0 deletions internal/app/connectconformance/server_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"connectrpc.com/conformance/internal"
conformancev1 "connectrpc.com/conformance/internal/gen/proto/go/connectrpc/conformance/v1"
"connectrpc.com/conformance/internal/tracer"
"google.golang.org/protobuf/proto"
)

Expand All @@ -51,6 +52,7 @@ func runTestCasesForServer(
errPrinter internal.Printer,
results *testResults,
client clientRunner,
tracer *tracer.Tracer,
) {
expectations := make(map[string]*conformancev1.ClientResponseResult, len(testCases))
for _, testCase := range testCases {
Expand Down Expand Up @@ -181,6 +183,7 @@ func runTestCasesForServer(
}
}

tracer.Init(req.TestName)
wg.Add(1)
err := client.sendRequest(req, func(name string, resp *conformancev1.ClientCompatResponse, err error) {
defer wg.Done()
Expand Down
3 changes: 2 additions & 1 deletion internal/app/connectconformance/server_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func TestRunTestCasesForServer(t *testing.T) {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
results := newResults(&testTrie{}, &testTrie{})
results := newResults(&testTrie{}, &testTrie{}, nil)

var procAddr atomic.Pointer[process] // populated when server process created
var actualSvrRequest bytes.Buffer
Expand Down Expand Up @@ -271,6 +271,7 @@ func TestRunTestCasesForServer(t *testing.T) {
discardPrinter{},
results,
&client,
nil,
)

if testCase.svrFailsToStart {
Expand Down
Loading
Loading