Add a context getter that returns with the type default value

[sagikazarmark]

Is your feature request related to a problem? Please describe. Currently context getters return with a value and an error. The error is nil, when the key can actually be found in the context. However, in most of the cases whether the value is there or not is irrelevant.

Furthermore, when a default is provided the error can just be ignored, since it will always be null.

Unfortunately, the error return value makes using the getters much harder.

Describe the solution you'd like I'd like to see a set of getters which:

• return the type default value when it's not found in the context
• optionally accepts a default value

For example: func GetStringWithDefault(key interface{}, default ...string) string

(Need a better name though)

Describe alternatives you've considered

Alternatively/in addition to the above, I could imagine a getter that panics/calls t.Fatal, for example:

func MustGetStrig(t testing.T, key interface{}, default ...string) string

(Again, name is a work in progress)

Additional context

Checking whether a key is present in the context or not can be a valid scenario, but the current implementation forces it onto the user and makes the test cases extremely verbose.

Consider the following (and let's say I do want the key to be checked):

id, err:= ctx.GetString("id")
if err != nil { t.Fatal(err) }
todo, err := fctx.Store.Get(context.Background(), id)
if err != nil {
    t.Fatal(err)
}

The current implementation forces me to manually handle the missing key scenario. In most of the cases people probably want to do two things:

• Ignore the fact that it's not there and just use the type default value
• Fail the test

The framework should support these scenarios better IMHO.

[bkielbasa]

Your point is valid IMO. Here's what I think:

• throwing a panic isn't an option. It can complicate things a lot. I've been there :)
• additional function for getting the variable OR fail the test sounds a good option.

I'd leave the current API as it is but add a set of new methods.

s := ctx.MustString("ffdsfa")

So, every method would start with word Must and then the type would come. The only exception would be Must which will return interface{} (similarly to ctx.Get()).

[sagikazarmark]

I'd leave the current API as it is but add a set of new methods.

That's what I had in mind too.

How will MustString behave? Must suggests that it will fail somehow if a value is not found.

A few alternative ideas:

• DefaultString
• GetStringD
• StringValue

[bkielbasa]

How will MustString behave? Must suggests that it will fail somehow if a value is not found.

It would fail the test: t.Fatal() would be called.

[bkielbasa]

speaking of defaults, why not leave it as it is right now? If we want to add a new set of methods for it, the last parameter in Get*() will make no sense any more. Then, I would go for removing it.

[sagikazarmark]

Leave what as it is?

[bkielbasa]

for now - yes. If there'll be more people complaining about it, we'll think about changing it. WDYT?

[sagikazarmark]

Sorry, I'm a bit confused about what you are suggesting exactly.

Are you suggesting to do nothing with this issue?

Personally, I don't care much about user-defined defaults, I care more about not having to handle every error manually.

[bkielbasa]

What I meant is that we can go with the Must* thing as we described but about defaults - let's leave it as it is. I think I need more time to think about it.

[sagikazarmark]

Sounds good to me.

How will the context receive the t instance? Should it be a parameter of Must*?

[bkielbasa]

the context is created at the very beginning https://github.com/go-bdd/gobdd/blob/master/gobdd.go#L255 so we can put it there in the New() function.

[sagikazarmark]

I'm not 100% that's true. For subtests the framework creates a new T, so using the root value will fail the wrong test.

So either each step receives a new context, with the correct T (totally viable IMHO, because the context is a single map) or Must functions must accept a T.

This gives me an idea though: what if the context indeed holds the correct instance of T and instead of passing it as an argument to a step function, we can get it from the context: ctx.T().

Testify suites work this way.