feat: table tests should point to test code #635
Conversation
Previously, ginkgo would identify the code locations of the failing Entry or DescribeTable as part of the ginkgo/extensions/table/table_entry.go or ginkgo/extensions/table/table.go files, respectively. When there are a large number of table entries, it is more desirable to provide the line numbers of the table and table entry directly to facilitate jumping to the relevant section of the test code. Fixes onsi#515
|
Hey @sethp-nr, Thanks a lot for your PR. |
| globalSuite.PushContainerNode(text, body, flag, codeLocation) | ||
| return true | ||
| } | ||
|
|
There was a problem hiding this comment.
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.
|
|
||
| Ω(output).Should( | ||
| ContainSubstring("failing_table_tests/failing_table_tests_test.go:12"), | ||
| "point to the failing assertion") |
There was a problem hiding this comment.
It's cool that you provide the optional test description given the complexity of the test.
| @@ -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 | |||
There was a problem hiding this comment.
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
Entryconstructor.
Although I've not seen people constructing the TableEntry directly, some people might do in which case we would break them with these changes.
| Focused bool | ||
| Flag types.FlagType | ||
|
|
||
| // TODO: what if this isn't set? |
There was a problem hiding this comment.
Good point. I don't have a clear answer. In #666 I just print the surrounding DescribeTable whenever it's not set.
| } else { | ||
| ginkgo.It(t.Description, body) | ||
| } | ||
| }, t.Flag, t.codeLocation, 0) |
|
That's all great feedback, thanks! It looks like your PR #666 captured all the valuable parts to me, let's close this in favor of that one :) |
Previously, ginkgo would identify the code locations of the failing
Entry or DescribeTable as part of the
ginkgo/extensions/table/table_entry.go or
ginkgo/extensions/table/table.go files, respectively.
When there are a large number of table entries, it is more desirable to
provide the line numbers of the table and table entry directly to
facilitate jumping to the relevant section of the test code.
Fixes #515