From f98eb987aa6fa5a177279ce7f393d4023415ca37 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sun, 25 Aug 2019 03:13:56 -0700 Subject: [PATCH] Remove "testing" import from public interface Currently, the DoTestCase, DoTestCaseFile, and DoTestCases functions, are exposed as part of the public interface. func DoTestCase(m Markdown, testCase MarkdownTestCase, t *testing.T) func DoTestCaseFile(m Markdown, filename string, t *testing.T) func DoTestCases(m goldmark.Markdown, cases []MarkdownTestCase, t *testing.T) Implementing these functions requires importing the `testing` package. Importing the `testing` package [automatically registers][1] a number of global [command line flags][2]. [1]: https://golang.org/src/testing/testing.go#L252 [2]: https://golang.org/cmd/go/#hdr-Testing_flags The effect of this is that any application using the standard `flag` package with goldmark will automatically get a number of unwanted command line flags. This is verifiable with the following program, package main import ( "flag" _ "github.com/yuin/goldmark" ) func main() { flag.Parse() } To fix this, the `testing` import needs to be removed from all non-test files. There are two ways to go about it, - If the functions are meant for external use, you can define an interface with a subset of the methods of `testing.T` and switch the functions to consume that. This is what testing libraries like [gomock] and [testify] do. - If the functions are meant for internal use, you can remove them from the public interface of the library and use them only from tests. [gomock]: https://godoc.org/github.com/golang/mock/gomock#TestReporter [testify]: https://godoc.org/github.com/stretchr/testify/require#TestingT Since these functions are meant to be used by external extensions, I've introduced a TestingT interface that is a subset of the functionality provided by `testing.T`. It supports the standard operations: logging, skiping, and failing tests, --- testutil.go | 18 +++++++++++++----- testutil_test.go | 7 +++++++ 2 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 testutil_test.go diff --git a/testutil.go b/testutil.go index 43d2d56..9f3ba52 100644 --- a/testutil.go +++ b/testutil.go @@ -4,14 +4,22 @@ import ( "bufio" "bytes" "fmt" - "github.com/yuin/goldmark/util" "os" "runtime/debug" "strconv" "strings" - testing "testing" + + "github.com/yuin/goldmark/util" ) +// TestingT is a subset of the functionality provided by testing.T. +type TestingT interface { + Logf(string, ...interface{}) + Skipf(string, ...interface{}) + Errorf(string, ...interface{}) + FailNow() +} + type MarkdownTestCase struct { No int Markdown string @@ -21,7 +29,7 @@ type MarkdownTestCase struct { const attributeSeparator = "//- - - - - - - - -//" const caseSeparator = "//= = = = = = = = = = = = = = = = = = = = = = = =//" -func DoTestCaseFile(m Markdown, filename string, t *testing.T) { +func DoTestCaseFile(m Markdown, filename string, t TestingT) { fp, err := os.Open(filename) if err != nil { panic(err) @@ -77,13 +85,13 @@ func DoTestCaseFile(m Markdown, filename string, t *testing.T) { DoTestCases(m, cases, t) } -func DoTestCases(m Markdown, cases []MarkdownTestCase, t *testing.T) { +func DoTestCases(m Markdown, cases []MarkdownTestCase, t TestingT) { for _, testCase := range cases { DoTestCase(m, testCase, t) } } -func DoTestCase(m Markdown, testCase MarkdownTestCase, t *testing.T) { +func DoTestCase(m Markdown, testCase MarkdownTestCase, t TestingT) { var ok bool var out bytes.Buffer defer func() { diff --git a/testutil_test.go b/testutil_test.go new file mode 100644 index 0000000..2ab24a6 --- /dev/null +++ b/testutil_test.go @@ -0,0 +1,7 @@ +package goldmark + +import "testing" + +// This will fail to compile if the TestingT interface is changed in a way +// that doesn't conform to testing.T. +var _ TestingT = (*testing.T)(nil)