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

Print correct code location of failing table test #666

Merged
merged 4 commits into from
May 27, 2020

Conversation

ansd
Copy link
Collaborator

@ansd ansd commented Apr 18, 2020

See #515 and #635 for motivation.

Given a test file

  1 package tabletests_test
  2
  3 import (
  4   . "github.com/onsi/ginkgo"
  5   . "github.com/onsi/ginkgo/extensions/table"
  6   . "github.com/onsi/gomega"
  7 )
  8
  9 var _ = Describe("Bar", func() {
 10   DescribeTable("table",
 11     func(i int) {
 12       Expect(i).To(Equal(0))
 13     },
 14     Entry("yolo1", 0),
 15     Entry("yolo2", 1),
 16     TableEntry{
 17       Description: "yolo3",
 18       Parameters:  []interface{}{1},
 19       Pending:     false,
 20       Focused:     false,
 21     },
 22   )
 23 })

Output before this PR:

$ ginkgo .
Running Suite: Tabletests Suite
===============================
Random Seed: 1587237068
Will run 3 of 3 specs

•
------------------------------
• Failure [0.000 seconds]
Bar
/Users/david/scratch/tabletests/bar_test.go:9
  table
  /Users/david/go/pkg/mod/github.com/onsi/ginkgo@v1.12.0/extensions/table/table.go:92
    yolo2 [It]
    /Users/david/go/pkg/mod/github.com/onsi/ginkgo@v1.12.0/extensions/table/table_entry.go:43

    Expected
        <int>: 1
    to equal
        <int>: 0

    /Users/david/scratch/tabletests/bar_test.go:12
------------------------------
• Failure [0.000 seconds]
Bar
/Users/david/scratch/tabletests/bar_test.go:9
  table
  /Users/david/go/pkg/mod/github.com/onsi/ginkgo@v1.12.0/extensions/table/table.go:92
    yolo3 [It]
    /Users/david/go/pkg/mod/github.com/onsi/ginkgo@v1.12.0/extensions/table/table_entry.go:43

    Expected
        <int>: 1
    to equal
        <int>: 0

    /Users/david/scratch/tabletests/bar_test.go:12
------------------------------

Output after this PR:

Running Suite: Tabletests Suite
===============================
Random Seed: 1587237050
Will run 3 of 3 specs

•
------------------------------
• Failure [0.000 seconds]
Bar
/Users/david/scratch/tabletests/bar_test.go:9
  table
  /Users/david/scratch/tabletests/bar_test.go:10
    yolo2 [It]
    /Users/david/scratch/tabletests/bar_test.go:15

    Expected
        <int>: 1
    to equal
        <int>: 0

    /Users/david/scratch/tabletests/bar_test.go:12
------------------------------
• Failure [0.000 seconds]
Bar
/Users/david/scratch/tabletests/bar_test.go:9
  table
  /Users/david/scratch/tabletests/bar_test.go:10
    yolo3 [It]
    /Users/david/scratch/tabletests/bar_test.go:10

    Expected
        <int>: 1
    to equal
        <int>: 0

    /Users/david/scratch/tabletests/bar_test.go:12
------------------------------

Notes:

  1. If the user constructs the TableEntry directly instead of using one of the (F/P/X)Entry constructors (as done in the yolo3 example above), the code location of the failing TableEntry defaults to the surrounding DescribeTable since I didn't find a way to programmatically locate the TableEntry. If someone has a good idea, let me know.
  2. I tried to be as much backwards compatible as possible. Although, I've not seen users constructing a TableEntry directly (most users use the (F/P/X)Entry constructors), the changes in here are backwards compatible if the users use a TableEntry composite literal with keyed fields. If however, users use a TableEntry with unkeyed fields (i.e. something like TableEntry{"yolo3", []interface{}{1}, false, false}), the changes in this PR will break them. In that case, they would need to name the fields or use the (F/P/X)Entry constructors.

Co-authored-by: David Ansari <dansari@pivotal.io>
@blgm
Copy link
Collaborator

blgm commented May 27, 2020

I think I'm ok with TableEntry{"yolo3", []interface{}{1}, false, false} breaking because although it's possible to use Ginkgo like this, it's not the documented way, and it's not best practice to construct a struct this way as the definition is outside of your control.

Copy link
Collaborator

@blgm blgm left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants