onsi / ginkgo

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

Support customizing the SpecContext #1363

Closed Airblader closed 4 months ago

Airblader commented 4 months ago

TL;DR We're wondering whether it'd be possible to somehow support customizing the SpecContext Ginkgo passes along.

In our application, we use LaunchDarkly to integrate feature flags. The corresponding client is added into the application context on startup (lets put aside for a second whether contexts are made for this šŸ™). In our unit tests, we of course do not start the entire application, so we usually have to add a (mock) client into the context, which is quite repetitive. Unfortunately, there's no single entrypoint to just add a dummy client for all tests.

Since we switched to using SpecContext wherever possible, we're wondering if there'd be a (good) way to allow us to customize how this context is created? I don't have a specific proposal for the API here, though.

Note that I'm using LaunchDarkly as our real-life example, but of course this generalizes to other things.

onsi commented 4 months ago

hey there - setting aside contexts (which, fwiw, iā€™m happy to use and mis-use to our heartsā€™ content - whatever gives us a clean and pragmatic pattern to get things done). The way Iā€™d approach this with Ginkgo is a top-level BeforeEach and a shared variable:

// this could go in your suite file
var client *MockClient

var _ = BeforeEach(func() {
    client = NewMockClient()
    DeferCleanup(client.Teardown)
})

and then just pass the client around as needed in my Its. I assume youā€™re already doing this and the issue is you donā€™t want to keep repeating:

It(ā€œdoes stuffā€, func(sc SpecContext) {
    ctx := context.WithValue(sc, ā€œclientā€, client)
    app := MakeApp(ctx)
})

Teaching Ginkgo to customize SpecContext will be complex - in part because users might need to control when this happens (globally? only after a variable is initialized? just for some tests?). But if I were faced with this Iā€™d either change the signature of MakeApp to take a context or a client (yuck) or Iā€™d add a simple helper to my test suite (again - probably in the suite file):

func MakeMockApp(ctx context.Context) *App {
    ctx := context.WithValue(sc, ā€œclientā€, client)
    return MakeApp(ctx)
}

note that I donā€™t need to pass client in since the global mock client is already there and initialized whenever MakeMockApp is called. Alternatively you could pass the client in (perhaps as an optional variadic argument) or you could have MakeMockApp make and return both the app and its mock client - lots of possibilities!

Two last thoughts:

  1. I imagine you are aware of this already but it bears repeating just in case: the lifecycle of SpecContext is tied to the node that it is injected into. So if you use it in a BeforeEach it gets cancelled when the BeforeEach ends. If you need a context that survives for the whole lifecycle of a spec (across all BeforeEach, It, and clean up nodes) youā€™ll want:
BeforeEach(func() {
   ctx, cancel := context.WithCancel(context.Background())
   //use ctx
   DeferCleanup(cancel)
})
  1. More philosophical - feel free to pipe to /dev/null but on this: ā€œin our unit tests, we of course do not start the entire applicationā€ - Iā€™m a fan of questioning that assumption. Of course, sometimes the answer is ā€œuse a mockā€ (though I find I never use a mocking library, instead I build my own fakes and give them some helpful test-oriented semantics and a family of custom matchers)ā€¦. but Iā€™ve found over the years that you can get pretty far (especially with go) spinning up the entire stack and relying on Ginkgo to parallelize things for you to retain performance. The beauty of this approach is you no logner need to fiddle with mocks and return values but, instead, get to play with your system as designed. And it makes refactoring vastly easier - instead of touching your specs to change the mocks the tests Just Workā„¢ and give you a sense of confidence that the refactor was successgful.
Airblader commented 4 months ago

Thanks for the response & apologies for my delayed one! Really appreciate how speedy you always are. We're great fans of Ginkgo in our project!

I assume youā€™re already doing this and the issue is you donā€™t want to keep repeating

Essentially, yes.

in part because users might need to control when this happens (globally? only after a variable is initialized? just for some tests?)

Yes, admittedly this is a difficult challenge. šŸ˜ž I'm happy to view this issue as a way to measure interest, or flat out close it if you think it's too much trouble.

In our case, we have something like a commontesting.NewContext() which now wraps this (among other things), and that happens to work for us, because we only use this in a BeforeEach, typically, to launch (parts of) the app, where we cannot use SpecContext anyway (as you also outlined). This suggestion was just an idea that popped up during a review.

onsi commented 4 months ago

got it, thanks. yeah iā€™m just not sure that thereā€™s gonna be a one-size-fits-all pattern that will make life uniformly easier for everyone. iā€™d rather just focus on making sure Ginkgo has the building blocks you need to accomplish this stuff with a little bit of glue code. itā€™s a bit long, but this recent issue #1366 is another example that shows some of the complexity we discussed here.

Airblader commented 4 months ago

That's totally fair šŸ‘ Let's just close this one then.

Airblader commented 2 weeks ago

@onsi This just came up again in our project by someone else. It's a long shot, but an idea that just came to me would be something like

var _ = Describe("", func() {
    BeforeEach(func() {
        ctx := MyNewContext()
        ctx = context.WithValue(ctx, "foo", "bar")

        UseContext(ctx)
    })

    It("", func(ctx SpecContext) {
        Expect(ctx.Value("foo")).To(Equal("bar"))
    })
})

Maybe I dream too much and this isn't even possible, but just throwing it out there.