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

feat: table tests should point to test code #635

Closed
wants to merge 2 commits into from
Closed
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
36 changes: 12 additions & 24 deletions extensions/table/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"reflect"

"github.com/onsi/ginkgo"
"github.com/onsi/ginkgo/internal/codelocation"
"github.com/onsi/ginkgo/types"
)

/*
Expand Down Expand Up @@ -42,57 +44,43 @@ It's important to understand that the `Describe`s and `It`s are generated at eva
Individual Entries can be focused (with FEntry) or marked pending (with PEntry or XEntry). In addition, the entire table can be focused or marked pending with FDescribeTable and PDescribeTable/XDescribeTable.
*/
func DescribeTable(description string, itBody interface{}, entries ...TableEntry) bool {
describeTable(description, itBody, entries, false, false)
describeTable(description, itBody, entries, types.FlagTypeNone)
return true
}

/*
You can focus a table with `FDescribeTable`. This is equivalent to `FDescribe`.
*/
func FDescribeTable(description string, itBody interface{}, entries ...TableEntry) bool {
describeTable(description, itBody, entries, false, true)
describeTable(description, itBody, entries, types.FlagTypeFocused)
return true
}

/*
You can mark a table as pending with `PDescribeTable`. This is equivalent to `PDescribe`.
*/
func PDescribeTable(description string, itBody interface{}, entries ...TableEntry) bool {
describeTable(description, itBody, entries, true, false)
describeTable(description, itBody, entries, types.FlagTypePending)
return true
}

/*
You can mark a table as pending with `XDescribeTable`. This is equivalent to `XDescribe`.
*/
func XDescribeTable(description string, itBody interface{}, entries ...TableEntry) bool {
describeTable(description, itBody, entries, true, false)
describeTable(description, itBody, entries, types.FlagTypePending)
return true
}

func describeTable(description string, itBody interface{}, entries []TableEntry, pending bool, focused bool) {
func describeTable(description string, itBody interface{}, entries []TableEntry, flag types.FlagType) {
itBodyValue := reflect.ValueOf(itBody)
if itBodyValue.Kind() != reflect.Func {
panic(fmt.Sprintf("DescribeTable expects a function, got %#v", itBody))
}

if pending {
ginkgo.PDescribe(description, func() {
for _, entry := range entries {
entry.generateIt(itBodyValue)
}
})
} else if focused {
ginkgo.FDescribe(description, func() {
for _, entry := range entries {
entry.generateIt(itBodyValue)
}
})
} else {
ginkgo.Describe(description, func() {
for _, entry := range entries {
entry.generateIt(itBodyValue)
}
})
}
ginkgo.ExplicitContainerNode(description, func() {
for _, entry := range entries {
entry.generateIt(itBodyValue)
}
}, flag, codelocation.New(2))
}
30 changes: 14 additions & 16 deletions extensions/table/table_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"reflect"

"github.com/onsi/ginkgo"
"github.com/onsi/ginkgo/internal/codelocation"
"github.com/onsi/ginkgo/types"
)

/*
Expand All @@ -12,13 +14,15 @@ TableEntry represents an entry in a table test. You generally use the `Entry` c
type TableEntry struct {
Description string
Parameters []interface{}
Pending bool
Focused bool
Flag types.FlagType
Copy link
Collaborator

@ansd ansd Apr 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also first thought it's a good idea to use the single Flag field instead of Pending and Focused. I'm not sure since I haven't written the original code but it looks like the Pending and Focused fields are there on purpose since they are exported to the user?
Also, if we do this change, we will break all users who construct a TableEntry directly (rather than using the (F/P/X)Entry constructors). Looking at the go docs for TableEntry, they state

TableEntry represents an entry in a table test. You generally use the Entry constructor.

Although I've not seen people constructing the TableEntry directly, some people might do in which case we would break them with these changes.


// TODO: what if this isn't set?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I don't have a clear answer. In #666 I just print the surrounding DescribeTable whenever it's not set.

codeLocation types.CodeLocation
}

func (t TableEntry) generateIt(itBody reflect.Value) {
if t.Pending {
ginkgo.PIt(t.Description)
if t.Flag == types.FlagTypePending {
ginkgo.ExplicitItNode(t.Description, func() {}, types.FlagTypePending, t.codeLocation, 0)
return
}

Expand All @@ -33,15 +37,9 @@ func (t TableEntry) generateIt(itBody reflect.Value) {
}
}

body := func() {
ginkgo.ExplicitItNode(t.Description, func() {
itBody.Call(values)
}

if t.Focused {
ginkgo.FIt(t.Description, body)
} else {
ginkgo.It(t.Description, body)
}
}, t.Flag, t.codeLocation, 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last 0 argument does not look correct?
You pass it further to ExplicitItNode where it becomes the timeout for PushItNode which would result in a timeout of 0 seconds here rather than the default timeout of 1 second defined here.

}

/*
Expand All @@ -53,26 +51,26 @@ Subsequent parameters are saved off and sent to the callback passed in to `Descr
Each Entry ends up generating an individual Ginkgo It.
*/
func Entry(description string, parameters ...interface{}) TableEntry {
return TableEntry{description, parameters, false, false}
return TableEntry{description, parameters, types.FlagTypeNone, codelocation.New(1)}
}

/*
You can focus a particular entry with FEntry. This is equivalent to FIt.
*/
func FEntry(description string, parameters ...interface{}) TableEntry {
return TableEntry{description, parameters, false, true}
return TableEntry{description, parameters, types.FlagTypeFocused, codelocation.New(1)}
}

/*
You can mark a particular entry as pending with PEntry. This is equivalent to PIt.
*/
func PEntry(description string, parameters ...interface{}) TableEntry {
return TableEntry{description, parameters, true, false}
return TableEntry{description, parameters, types.FlagTypePending, codelocation.New(1)}
}

/*
You can mark a particular entry as pending with XEntry. This is equivalent to XIt.
*/
func XEntry(description string, parameters ...interface{}) TableEntry {
return TableEntry{description, parameters, true, false}
return TableEntry{description, parameters, types.FlagTypePending, codelocation.New(1)}
}
16 changes: 16 additions & 0 deletions ginkgo_dsl.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,22 @@ func AfterEach(body interface{}, timeout ...float64) bool {
return true
}

//ExplicitItNode adds a fully-specified It node to the global context. This function is largely only
//useful for meta-programming or adding on to the Ginkgo DSL, when writing normal test cases it's best
//to use one of the others.
func ExplicitItNode(text string, body interface{}, flag types.FlagType, codeLocation types.CodeLocation, timeout time.Duration) bool {
globalSuite.PushItNode(text, body, flag, codeLocation, timeout)
return true
}

//ExplicitContainerNode adds a fully-specified Container node to the global context. This function is
//largely only useful for meta-programming or adding on to the Ginkgo DSL, when writing normal test
//cases it's best to use one of the others.
func ExplicitContainerNode(text string, body func(), flag types.FlagType, codeLocation types.CodeLocation) bool {
globalSuite.PushContainerNode(text, body, flag, codeLocation)
return true
}

Copy link
Collaborator

@ansd ansd Apr 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although exporting these two functions works, it's not ideal. Every user which imports github.com/onsi/ginkgo will have access to these functions although these are only internal helper functions.

func parseTimeout(timeout ...float64) time.Duration {
if len(timeout) == 0 {
return time.Duration(defaultTimeout * int64(time.Second))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package failing_table_tests

import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"testing"
)

func TestFocused_fixture(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Failing_table_tests Suite")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package failing_table_tests

import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
)

var _ = Describe("FailingTableTests", func() {

DescribeTable("a failing entry",
func(val bool) { Ω(val).Should(BeTrue()) },
Entry("passing", true),
Entry("failing", false),
)
})
38 changes: 37 additions & 1 deletion integration/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import (
)

var _ = Describe("Running Specs", func() {
var pathToTest string
// Provide a default, else a BeforeEach with an accidental redeclaration (`:=`) will cause the integration test suite to recurse infinitely.
pathToTest := "invalid_path"

isWindows := (runtime.GOOS == "windows")
denoter := "•"
Expand Down Expand Up @@ -353,6 +354,41 @@ var _ = Describe("Running Specs", func() {
})
})

Context("when pointed at a packages with a failing table test", func() {
BeforeEach(func() {
pathToTest = tmpPath("failing_table_tests")
copyIn(fixturePath("failing_table_tests"), pathToTest, false)
})

It("should identify the failing entry", func() {
session := startGinkgo(pathToTest, "--noColor")
Eventually(session).Should(gexec.Exit(1))
output := string(session.Out.Contents())

Ω(output).Should(ContainSubstring("Test Suite Failed"))
Ω(output).Should(ContainSubstring(fmt.Sprintf("%s Failure", denoter)))

Ω(output).Should(ContainSubstring("Summarizing 1 Failure:"))
Ω(output).Should(ContainSubstring("[Fail] FailingTableTests a failing entry [It] failing"))

// The table
Ω(output).Should(
ContainSubstring("failing_table_tests/failing_table_tests_test.go:11"),
"point to the failing table")
Ω(output).ShouldNot(MatchRegexp(`github.com/onsi/ginkgo/extensions/table/table.go:\[\d+\]`))

// The entry
Ω(output).Should(
ContainSubstring("failing_table_tests/failing_table_tests_test.go:14"),
"point to the failing entry")
Ω(output).ShouldNot(MatchRegexp(`github.com/onsi/ginkgo/extensions/table/table_entry.go:\[\d+\]`))

Ω(output).Should(
ContainSubstring("failing_table_tests/failing_table_tests_test.go:12"),
"point to the failing assertion")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's cool that you provide the optional test description given the complexity of the test.

})
})

Context("when running recursively", func() {
BeforeEach(func() {
passingTest := tmpPath("A")
Expand Down