Request for feedback - GoConvey DSL v2.0

[mdwhatcott]

I've opened this issue as a way to talk about how the syntax of the DSL might be altered to provide solutions for some of our long-standing, known issues. Please provide feedback/ideas in the comments.

I'm going to present the two options I'm considering below. Feel free to vote for either or present your own. The goal with these changes is to eliminate all package-level state from GoConvey so that test functions can be run in parallel. In the two options below this is accomplished with a sort of Context object which I'm calling C. This object will house its own runner and a reporter (which are now package level resources). I've prototyped option 2 on the concurrency branch of this project.

Option 1:

Convey("Stuff", t, func(c *C) {
    c.Convey("More stuff", func() {
        c.So(1, ShouldEqual, 1)
    })

    c.SkipConvey("Skipped stuff", func() {
        c.So(1, ShouldEqual, 2)
    })

    c.Reset(func() {
        // stuff
    })
})

Option 2:

Convey("Stuff", t, func(c *C, so A) {
    Convey("More stuff", c, func() {
        so(1, ShouldEqual, 1)
    })

    SkipConvey("Skipped stuff", c, func() {
        so(1, ShouldEqual, 2)
    })

    Reset(c, func() {
        // stuff
    })
})

[jfbus]

+1 for option 1

[rmetzler]

Context sounds pretty generic. I wonder if it might be better to use tests or suite.
But as a non native English speaker I also wondered myself what So means. I guess it's one of these definitions.

[mdwhatcott]

@rmetzler - Good point, I'm still not sold on the name of the new struct either. suite is an option I'll consider--thanks for the suggestion. Just curious, of the two options presented above, which one do you prefer?

In the case of So, the definition from your link that corresponds with the intended meaning is:

and for this reason; therefore:

[jonnenauha]

Option 1

[peterhellberg]

I’m leaning towards Option 2.

[mdwhatcott]

Moving forward with option 2. Personally, I don't like having to see c. everywhere so that's the main reason for option 2 over option 1. More info: https://michaelwhatcott.com/coming-soon-goconvey-v2-0/

[mdwhatcott]

@smartystreets -

I was just about to pull the trigger for v2.0 with option 2 when "lightning struck my brain" (didn't even hurt). Try this on for size:

Suite(t, func(convey C, so So, reset R, focus Focus, skip S) {
    convey("nested 1", nil)

    focus("EXCLUSIVE!!", func() {
        convey("special stuff", nil)
    })

    skip("something else", func() {
        so(1, ShouldEqual, 1)
    })

    reset(func() {

    })
})

I know what you're thinking: "What!? Why do I have to pass in 4 arguments in the top-level Convey? Well, you don't! The only ones you'll be passing in regularly are the convey and the so. The others will be optional. Notice also, that we no longer have to pass a c in at every nested Convey. The new convey is basically a method on the old c, just like the so has been. And yup, you guessed it, focus and skip are also methods on c (Context). So, by allowing the func at the top-level to be one of various function types (yes, I'll account for variations in parameter ordering) you can pick and choose the tools you'll need for a specific test suite within a go test function just by adding them there.

Finally, did you notice the focus parameter? That's the "exclusive" convey that will aid in debugging. It will totally be possible using this proposed DSL. I've also got some other ideas for neat features that wouldn't be too hard (randomized execution, sequential execution, etc...).

I'd like to take some time tomorrow or Thursday to get your feedback and kick this around.

[mholt]

That. Would. Be. Awesome.

It is getting complicated quickly, though. Is the top-level Convey always going to be "Suite"? Can it stay "Convey" for simplicity?

[mdwhatcott]

Yes, it could (and it's probably still better that way). I couldn't tell what would be more confusing, having uppercase Convey for the top-level and lowercase convey for nested levels or having a different method for the top-level and nested levels. It's up for discussion. I'm also contemplating allowing this option as well:

func TestFeatureUnderDevelopment(t *testing.T) {
    Convey(t, func(c Context) {
        c.Convey("nested something", nil)

        c.Focus("EXCLUSIVE", func() {
            c.Convey("special stuff", nil)
        })

        c.Skipconvey("something else", func() {
            c.So(1, ShouldEqual, 1)
        })

        // usually needed only for integration tests
        c.Reset(func() {

        })
    })  
}

If that func signature can vary than why not just provide the raw context? I don't think the tests are as pretty overall that way but it could be an option. Basically, this becomes sort of a mix between the original option 1 and option 2. Then again, the more we allow, the crazier the code to support it (but I don't think it will be too bad).

[mholt]

Personally, I'm not liking the c. stuff as much. Using the code generator, the previous DSL (with all the stuff in the top-level convey/suite) is appealing, but the c. style is appealing when writing code manually: there's less to remember.

Another thought: what about, instead of Focus, we have OnlyConvey -- to be consistent with SkipConvey and Convey?

[mdwhatcott]

I see your point and I feel the same way. I'm wondering about giving people with other sensibilities a choice.

I'm also thinking of changing SkipConvey to just Skip so Focus (or Only) would be a standalone term then too. I've got ideas flying around like crazy. Nothing is settled yet.

[mdwhatcott]

Stop the presses! I just found a way to fix issue #70, which was one of the major motivations for a 2.0 version. The fix doesn't require any change to the DSL. Stay tuned.