f1 icon indicating copy to clipboard operation
f1 copied to clipboard

Make Error and Fatal functions compatible with testing.T

Open sirockin opened this issue 1 year ago • 3 comments

What

Making testing.Error and testing.Fatal functions compatible with testing.T by changing the single error argument to a variable argument list accepting any.

Why

Facilitates reusing test scenarios by extracting common testing interface

Testing

The only tests which currently exist for the two functions confirm that the methods fail the load test do not test the logging functionality. We update these with test cases to cover different arguments being passed in.

Example code

main.go:


package main

import (
	"github.com/form3tech-oss/f1/v2/pkg/f1"
	"github.com/form3tech-oss/f1/v2/pkg/f1/testing"
)

// TF is the interface common to T and testing.TB
type TF interface {
	Cleanup(fn func())
	Error(args ...any)
	Errorf(format string, args ...any)
	Fail()
	FailNow()
	Failed() bool
	Fatal(args ...any)
	Fatalf(format string, args ...any)
	Log(args ...any)
	Logf(format string, args ...any)
	Name() string
}

// Check T implements TF
var _ TF = (*testing.T)(nil)

func main() {
	f1.New().
		Add("Foo", setupFoo).
		Execute()
}

func setupFoo(t *testing.T) testing.RunFn {
	// Do once-only setup here

	return func(t *testing.T) {
		FooTest(t)
	}
}

// Foo can be run as a load test or a standard go test
func FooTest(TF) {
	// Do the test here
}

main_test.go:


package main

import (
	"testing"
)

// Run  FooTest as a standard go test 
func TestFoo(t *testing.T) {
	main.FooTest(t)
}

Resolves #278

sirockin avatar Sep 11 '24 08:09 sirockin

I'm surprised the CI pipeline is failing on interface compatibility given the changes are non-breaking.

sirockin avatar Sep 11 '24 14:09 sirockin

@sirockin This is a breaking change for any users that may have already done what we're trying here - create an abstraction to share code between f1 and other parts of their codebase.

We want to avoid breaking users as much as possible hence my comment on the other PR: https://github.com/form3tech-oss/f1/pull/279#issuecomment-2342601643

I'm open to ideas, but I still think adding a TB() method to T that returns a compatible object is a reasonable approach.

nvloff-f3 avatar Sep 12 '24 05:09 nvloff-f3

@sirockin This is a breaking change for any users that may have already done what we're trying here - create an abstraction to share code between f1 and other parts of their codebase.

I may be being dense (it's early in the morning for me 😂 ) but I'm still not clear on why this would be a breaking change. The two methods will still behave exactly as previously for the only case which was previously compilable: a single error argument.

sirockin avatar Sep 12 '24 07:09 sirockin