onsi / ginkgo

A Modern Testing Framework for Go
http://onsi.github.io/ginkgo/
MIT License
8.37k stars 660 forks source link

Support BeforeAll/AfterAll in un-Ordered container #1103

Closed tzvatot closed 1 year ago

tzvatot commented 1 year ago

Using v2.1.4.

We like to use BeforeAll/AfterAll feature added recently. When using BeforeAll/AfterAll, it's enforcing Ordered container. We cannot use Ordered container, because using DescribeTable with multiple entries, if one entry fails, ginkgo will not run the following entries.

There's actually two options here: Either have DescribeTable continue to run the next entry if one fails: that will be a backward compatibility breaking change. Or, have BeforeAll/AfterAll not enforcing Ordered container, which I assume will allow DescribeTable to not skip the post-failure entries.

onsi commented 1 year ago

hey @tzvatot is it important or preferred that the table entries run in a random order? or be able to run in parallel?

also - can you share more about why you'd prefer to use a BeforeAll here instead of a BeforeEach? Is it for performance considerations?

BeforeAll/AfterAll only apply in Ordered containers because the complexity of orchestrating and reasoning about BeforeAll/AfterAll goes up when you allow the associated specs to run in parallel and be randomized amongst the rest of the suite. Im hesitant to add that complexity at this time.

However there certainly are cases when you don't care about parallelism or randomization and just want the Ordered container to continue if failures occur instead of aborting. I'm open to adding support for that, probably in the form of an additional decorator like ContinueOnFailure. It sounds like that would satisfy your usecase?

With Christmas and a New Years coming up I don't have a timeline for when I'd get to it, though.

tzvatot commented 1 year ago

I'll start from the end: Yes, ContinueOnFailure decorator would be a good solution.

The issue in general can be described by this test as an example:

var _ = Describe("Ordered Test", Ordered, func() {
    BeforeAll(func() {
             // do some initialization
    })

    Context("1", func() {
        It("Foo", func() {
            Fail("Should fail")
        })

        It("Bar", func() {
            fmt.Println("Should succeed")
        })
    })
})

I would expect test "Bar" to be successful and not skipped due to the previous test failing.

The same apply with DescribeTable:

var _ = Describe("Ordered Test", Ordered, func() {
    BeforeAll(func() {
            // initialization
    })

    Context("1", func() {
        DescribeTable("Foo",
            func(something string) {
                if something == "abc" {
                    Fail("failed")
                } else {
                    fmt.Println("success")
                }
            },
            Entry("Foo: Entry 1", "abc"),
            Entry("Foo: Entry 2", "def"))
    })
})

The fact that entry 2 is skipped is a bug IMO.

For my own use case: I don't require parallel execution or randomization at this time. Might be useful in the future though.

The reason to use BeforeAll and not BeforeEach is that if I'm doing some initialization that should prepare resources for all the tests, it should be done once for all of them. Doing it per test is not efficient, both in time and money. For example, deploying a complete openshift cluster can take 40 minutes, and there's no point to prepare one per It, as it will take forever to run.

onsi commented 1 year ago

I'll work on ContinueOnFailure at some point and let you know when it's ready.

onsi commented 1 year ago

I just shiped Ginkgo 2.7.0 which includes ContinueOnFailure. The only constraint is that you must put it on the outermost Ordered container. Please give it a try and take a look. Note that parallelization and randomization will not apply to specs in an Ordered container.

tzvatot commented 1 year ago

@onsi thanks! I've tried to use it. However, I'm getting this error:

 ContinueOnFailure can only decorate an Ordered container, and this Ordered
  container must be the outermost Ordered container.

This is a code example that generates this error:

var _ = FDescribe("Most outer container", Ordered, func() {
    Describe("Continue", ContinueOnFailure, func() {
        BeforeAll(func() {
            fmt.Println("BeforeAll")
        })
        Describe("Inner", func() {
            It("A", func() {
                fmt.Println("B test")
            })
            It("B", func() {
                fmt.Println("B test")
            })
        })
    })
})

What am I doing wriong?

onsi commented 1 year ago

You need to do this:

var _ = FDescribe("Most outer container", Ordered, ContinueOnFailure, func() {
    BeforeAll(func() {
        fmt.Println("BeforeAll")
    })
    Describe("Inner", func() {
        It("A", func() {
            fmt.Println("B test")
        })
        It("B", func() {
            fmt.Println("B test")
        })
    })
})

ie you apply ContinueOnFailure alongside Ordered

tzvatot commented 1 year ago

Works. Thanks @onsi !