Support custom feature context

[sagikazarmark]

Is your feature request related to a problem? Please describe.
The context is great for primitive types, but not that great for complex types:

myService := ctx.Get("myservice").(*MyService)
myOtherService := ctx.Get("myotherservice").(*MyOtherService)

Describe the solution you'd like
It would be nice if the step function could accept an arbitrary contex and "unmarshal" the internal context on it. For example, by having to implement an Unmarshal function on the struct:

type MyFeatureContext struct {
    MyService Service
}

func (c *MyFeatureContext) UnmarshalContext(ctx context.Context) {
   c.MyService = ctx.Get("myservice").(*Service)
}

Although this is not so much different from getting the values directly from the context, it keeps the step functions relatively clean.

Describe alternatives you've considered

As a practice, the following could work, minimizing the work on both ends:

myContext := ctx.Get("mycontext").(*MyContext)

myContext.MyService.Do()

This could even go to the documentation if it sounds an acceptable practice (not sure yet to be honest).

[bkielbasa]

I think it's a similar situation to #55.

The biggest issue with this suggestion is that we don't use the custom context any more. I have to update the docs :)

The only thing that comes to my mind is some code generation but don't have any specific idea right now.

[sagikazarmark]

I wasn't sure if I liked the custom context at first, but I think it was an interesting and useful concept. IMHO it's much harder to pack/unpack values from/into the official context implementation, and I actually think that it's not really a good fit for this package.

Context is useful when you want to take some values across APIs, but don't want to add them to the API. In this case however, "context" values are very much part of the API.

I guess this should probably be a separate issue, but the bottom line is, using context.Context seems way more cumbersome.

It might get easier though with some syntactic sugar. I don't know.

[bkielbasa]

how about writing custom helpers?

func getServiceX(ctx context.Context) {
   return ctx.Get("myservice").(*Service)
}

s := getServiceX(ctx)

I don't feel that extending the context is the best approach here.

[sagikazarmark]

I don't think the context has to be extended.

Here is what I had in mind:

Suppose I have a custom context:

type MyFeatureContext struct {
    MyService Service
}

If it implements the following function:

func (c *MyFeatureContext) UnmarshalContext(ctx context.Context) error {
   c.MyService = ctx.Get("myservice").(*Service)

   return nil
}

Then a step function could have the following signature:

func myStep(ctx MyFeatureContext, var1 string) error {}

It probably sounds more complicated than it actually is. It basically requires checking the context type of every step function and whether that type implements an interface or not. It requires reflection though, but I don't think that's a problem.

Although custom helpers would definitely help, supporting custom contexts makes the steps more expressive IMHO.

On the other hand: my current proposal doesn't solve setting anything in the context

[bkielbasa]

I think it will complicate things a lot. Please notice that the context is created for every feature. Your step may require a specific type (struct or interface) but in one of the features where you reuse the step, you don't change the context type or pass incompatible one.

we can wait for other opinions about that but for me, this code is more clear and idiomatic (at least for me)

func TestXYZ(t *testing.T) {
    s := getServiceCtx(ctx)
}
func getServiceCtx(ctx context.Context) {
   return ctx.Get("myservice").(*Service)
}

[sagikazarmark]

What exactly does that make more idiomatic?

Let's put go-bdd aside for a moment, and just look at the context thing.

Here is an example for using the current context:

func myFunc(ctx context.Context) error {
    service := ctx.Get("myservice").(*Service)
    someOtherStuff := ctx.Get("otherstuff").(*OtherStuff)

    return service.DoSomething(someOtherStuff)
}

func myFunc2(ctx context.Context) error {
    service := ctx.Get("myservice").(*Service)
    someOtherStuff := ctx.Get("otherstuff").(*OtherStuff)

    return service.DoSomething(someOtherStuff)
}

func myOtherFunc(ctx context.Context) error {
    service := ctx.Get("myotherservice").(*OtherService)

    return service.DoSomethingElse()
}

And here is one with a custom context:

type MyContext struct {
    Service *Service
    SomeOtherStuff *OtherStuff

    OtherService *OtherService
}

func myFunc(ctx MyContext) error {
    return ctx.Service.DoSomething(ctx.SomeOtherStuff)
}

func myFunc2(ctx MyContext) error {
    return ctx.Service.DoSomething(ctx.SomeOtherStuff)
}


func myOtherFunc(ctx MyContext) error {
    return ctx.OtherService.DoSomethingElse()
}

The key differences:

• the custom context makes dependencies more obvious for steps
• the custom context provides more safety with compile time type checks
• the custom context removes some repetition
• the custom context (in my opinion) makes the steps easier to read (the code is not obfuscated by type assertions that has nothing to do with the step itself, but is a byproduct of using an untyped context)

For me the context is the necessary evil, because you have to carry all sorts of information in a "request" like object, but ultimately the context requires you to build a larger mental model of everything in your head, and know that in step 1 you added something (with xyz key) that you might want to retrieve in step 9.

So far you mentioned complexity of the implementation, and that's a perfectly valid reason (although I still think that it sounds a lot more complex at first than it actually is).

But you also brought up it being idiomatic and with that I completely disagree. 🙂 (Then again, idiomatic could also be a synonym to subjective)

I've seen the custom context thing implemented in a few places (eg. Behat, which I think is an exceptionally good BDD framework).

I could also agree that this feature could be added post-1.0, since it's totally backward compatible and until then this simpler concept of context could help stabilizing the library itself. But I'd keep this issue open, probably take a look at other implementations, reach out to the community for their opinion.

[bkielbasa]

I think I see your point.

I can transform your example to this:

func getMyService(ctx context.Context) *Service {
    return ctx.Get("myservice").(*Service)
}
func myFunc(ctx context.Context) error {
    service := getMyService(ctx)
    someOtherStuff := ctx.Get("otherstuff").(*OtherStuff)

    return service.DoSomething(someOtherStuff)
}

func myFunc2(ctx context.Context) error {
    service := getMyService(ctx)
    someOtherStuff := ctx.Get("otherstuff").(*OtherStuff)

    return service.DoSomething(someOtherStuff)
}

func myOtherFunc(ctx context.Context) error {
    service := getOtherService(ctx)

    return service.DoSomethingElse()
}

It removes the repetition. About the compile-time checks - I'm not sure about that. The context has to be built in the runtime and depending on BeforeStep/or previous steps those fields may or may not be filled in. Can be nil as well. The same problem is with custom helpers I suggested.

Another problem is that the custom context cannot be a struct. It has to be an interface. Otherwise, how do we know how to build the custom struct? The only way to resolve the issue is to pass an empty custom context at the very beginning of the test and reuse it in every feature. That would make sense and would work. But it means that every step in the feature has to accept the same context (struct or interface).

It has implications:

• we have the testhttp package which won't be compatible with your custom context. I'm planning to add more of them.
• the context.Context has to be refactored to the interface (what's actually a good thing :) )
• if the step doesn't accept the custom context, it will fail. We cannot detect it in the compile-time
• I guest some other I cannot find right now

I'm not saying no to this feature but I'm looking for as simple solution as possible. There are some scenarios we have to think about in advance because it can make things even more complicated.

[sagikazarmark]

Another problem is that the custom context cannot be a struct.

Correction: it has to implement an interface.

But it means that every step in the feature has to accept the same context (struct or interface).

I don't think that's true. Internally, the framework should maintain a context.Context and sort of unpack it into a custom context, whenever it is required by a step and probably "pack" it, after the step is done. Which (I guess) renders the implications you listed invalid.

Here is what I have in mind (demonstrating with pseudo code):

func (s *suite) addStep(fn stepFunc) {
    // validate that the stepFunc has a context
    // that implements our ContextMarshaler interface
    // which is able to unpack values into the custom context
}

func (s *suite) run() {
    ctx := NewContext()

    for _, step := range steps {
        if stepAcceptsCustomContext(step) {
            // This should probably be a pointer
            customCtx := createCustomContext(step)

            // unmarshal values to the custom context
            customCtx.UnmarshalContext(ctx)

            callStepWithCustomContext(ctx, step)

            // copy back any changes to the context
            customCtx.Marshal(ctx)
        } else {
            callStepWithRegularContext(ctx, step)
        }
    }
}