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

Feature requests: more structured tests and names for Ginkgo #144

Closed
filbranden opened this issue Feb 25, 2015 · 8 comments
Closed

Feature requests: more structured tests and names for Ginkgo #144

filbranden opened this issue Feb 25, 2015 · 8 comments

Comments

@filbranden
Copy link

As referenced in kubernetes/kubernetes#3837, it would be nice to be able to have a more structured way to associate tags or a name with specific tests, in case we only want to run a subset of the tests or a single test.

Maybe make it more structured as a []string or add a new "verb" (like Describe, It etc.) that you could use to associate tags to existing tests?

Ideally a unique name, though if we have tags we can always use name=TestThisAndThat which is unique enough...

@onsi @rrati @timothysc @zmerlynn @roberthbailey @satnam6502 @lavalamp

@onsi
Copy link
Owner

onsi commented Feb 28, 2015

I'm trying to come up with a way to express this that is backwards compatible and not ugly. The best I can come up with is something like:

Describe("User Propagation", func() {
    BeforeEach(func() {
        ...
    })

    It("should do something", func() {
        Ω(...).Should(...)
    })

    It("should do something else", func() {
        Ω(...).Should(...)
    }).Label("slow")

    Context("when connected to the internet", func() {
        It("should do something special", func() {
            Ω(...).Should(...)
        })

        It("should do something expensive", func() {
            Ω(...).Should(...)
        }).Label("slow")
    }).Label("requires-internet")
}).Label("complex")

Then the CLI would support:

ginkgo --labelsMatch="!requires-internet,!slow"

(this runs all tests that don't have the "requires-internet" and "slow" labels)

ginkgo --labelsMatch="requires-internet,!slow"

(this runs all tests that have "requires-internet" and not "slow") - etc...

I'd then update the default report to include tag information when running in verbose mode.

You'd also be able to mix in --focus and --skip -- everything gets basically ANDed together.

The only thing about this I don't like is that Label appears at the end of the It/Describe -- which means a long test will only have the label near the bottom.

Alternatives could look like:

Label([]string{"foo"}, Describe("my description", func() {

})

Or

LabelledDescribe("my description", []string{"foo"}, func() {

})

But those strike me as ugly...

Any thoughts?

@onsi
Copy link
Owner

onsi commented Mar 18, 2015

ping? any thoughts on this approach?

@rrati
Copy link

rrati commented Mar 19, 2015

I like this proposal. It allows you to more easily select a set of test cases to run and more importantly allow you to filter tests you don't want. Unless I'm missing something, one of the missing test selections in ginkgo is the ability to have a test in a suite but not have it run by default. The only way I see to handle that is to make the test/suite Pending, but that then involves manually changing the file to allow it to run. I don't see a way to say "run the suite except for these tests" from the command line. You could focus on the exact tests you want, but really I'm wanting to exclude tests.

This proposal provides nice tags you can mix and match and better control which tests are run. Would you allow multiple label definitions per test/suite? Something like:

Describe("User Propagation", func() { }).Label("slow", "network", "performance")

I also dislike the labels being at the end, but I think they look cleaner then the proposed pre-fix options.

@onsi
Copy link
Owner

onsi commented Mar 19, 2015

Unless I'm missing something, one of the missing test selections in ginkgo is the ability to have a test in a suite but not have it run by default.

It sounds like you actually want to have separate test suites. Wanting a single test suite to represent two or three different classes/kinds of test is an anti-pattern (though it can be accomplished with the proposed Labels API and, even today, if you're willing to put up with the "label" living in the It/Describe strings).

The only way I see to handle that is to make the test/suite Pending, but that then involves manually changing the file to allow it to run.

Pendings are meant to convey tests that are currently failing but that are meant to be fixed in the near-future. I'll often write a failing spec for a bug and mark it Pending until I can resolve the bug. You are right, it's not meant to select subsets of the test dynamically.

I don't see a way to say "run the suite except for these tests" from the command line. You could focus on the exact tests you want, but really I'm wanting to exclude tests.

You can user ginkgo --skip="regex" (documented here).

I often use this as follows:

Describe("Copying a file into the cluster {LOCAL}", func() {
    It("should copy the file in", func() {
        .... // does something that only works locally
    })
})

Describe("Uploading a file to the cluster", func() {
    It("should upload the file", func() {
        .... // does something that works locally and/or against a remote cluster
    })
})

When I run the tests locally I just ginkgo.
When I run the tests on CI I have my test script run ginkgo --skip="{LOCAL}"

Because of this flexibility I haven't needed the proposed .Label API. It doesn't add very much and I find the existing behavior to be fairly flexible. Since --skip takes a regexp you can do things like ginkgo --skip="{LOCAL}|{STRESS}" etc...

@onsi
Copy link
Owner

onsi commented Mar 28, 2015

Not intending to drag my feet on this. It sounds like there was a bit of confusion as to what Ginkgo gives you out of the box. In light of my previous post describing --skip and the user of regexes and {FOO} "tags" do y'all think there's much value in a more formal way of specifying per test labels? It's starting to feel a little YAGNI to me -- are there problems that exist today that it does not solve? Or is the user of {FOO} too much of a smell?

@filbranden
Copy link
Author

Sorry for the delay, I was on vacation for a couple of weeks.

I'm not really sure we can find a good enough way to introduce this feature given the current constraints...

At this point I think my better suggestion is to keep this one in mind so that if at some point you decide to change the API (maybe on a ginkgo2 package or such?) then try to address it then...

Otherwise, I kind of agree that using {LOCAL} etc. tags embedded in the text is probably better than the .Label(...) at the end of the Describe().

So I'm leaning towards closing this one and will do so... If anyone disagrees, please feel free to reopen it.

Cheers,
Filipe

@ixdy
Copy link
Contributor

ixdy commented Oct 9, 2015

We've been tagging our tests using "[tag]" in the name for a while now, which basically seems to work, but it interacts poorly with the JUnit reporter and CI systems - every time we add or remove the tag, we lose any history for that test, since the name is now different.

Do you have any suggestion for how we might approach this? One idea was passing some sort of regular expression to the JUnit reporter, which it would then use to strip tags from the name saved in the JUnit XML report, though that seems a little hacky.

@jianzhangbjz
Copy link
Contributor

I'd like to use the Lable flag to run specific tests. But, now, I couldn't find the --labelsMatch option.
So, how to run the specific tests? Only one way by now: -focus? @onsi

mac:~ jianzhang$ ginkgo version
Ginkgo Version 1.6.0

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

No branches or pull requests

5 participants