Is it possible to write my own decorator?

[markxnelson]

Hi, thanks for the great project!

I would like to write my own decorator, so I could do something like specifying the requirements for a particular node, and get control in my decorator handler code to "make right" before that node gets to execute. Is it possible to do something like this?

I know I could use a BeforeXX to do this kind of thing. I have some tests that have some timing/dependency requirements. I can do what I need with a structure like this:

SynchronizedBeforeSuite { create k8s cluster }
Describe(Ordered) {
  BeforeAll { install old version of my product }
  Context {
    It { test something }
    It { test something else }
  }
  It { run my product upgrade function }
}

Describe {
  BeforeEach { wait until the upgrade in the other spec finishes }
  It { test something }
  It { test something else }
}

This is a bit awkward/artificial - I have to run the two Describes in parallel, and the second one is really just waiting until the first one finished. This is important to me because in reality there are way more than two It nodes in there, and some of them take minutes to run, so I want to be able to run them in parallel. I cannot use one big describe, because once I decorate it with Ordered, everything in it is run in order. If it were possible to mark a node inside an ordered node as "parallel" that'd also solve my particular use case.

If I could provide my own decorators, I could do something like

Describe(RequiresUpgradeComplete) {
  It {}
  It {}
}

This would let me centralize that "wait until the upgrade in the other spec finishes" code and reuse it in many tests, rather than needing to put it BeforeEach's all over the place.

I guess my meta-need is to be able to put a decoration on a node that I can use to specify some arbitrary set of conditions that must be met before that node starts executing. Sorry if this is a bit of a ramble, hope to hear back from you with your thoughts.

By the way, the slack link on the main README does not work, it just takes you to a page that asks for the slack workspace name.

[onsi]

hey @markxnelson - one of Ginkgo's foundational assumptions is that your specs (i.e. the individual Its) are independent from each other. This enables parallelization and randomization, and encourages writing test suites that are less prone to flakiness and test pollution. The new Ordered decorator (which added a surprising amount of complexity to the codebase) begins to shift that assumption in a very controlled way to allow users to break up what is effectively a single test into multiple subunits - at this point I'm wary of adding any new functionality that further breaks the spec-independence contract as I believe it will make tests, especially complex integration tests, even harder to reason about (and the Ginkgo codebase substantially more complex).

Rather than explore your use-case in terms of solutions (decorates/arbitrary sets of conditions/etc.) I'd love to explore the problem more deeply with you to help point you at possible solutions Ginkgo and Gomega can provide today. Can you say more about your setup and what behavior you're trying to test? It sounds like you're testing how your produce behaves after an upgrade. And that you have a few basic tests to run on the old version followed by some extensive tests of the product after the upgrade, which you'd like to parallelize. Is that right? Are there other tests in the suite as well?

It would help to know about your k8s tenancy requirements. Can you have multiple instances of your product on the same k8s cluster at different version levels? Or do they each need a dedicated cluster?

I can think of a number of approaches to solve for this use case using what's in the box today... but knowing a bit more would help.

[markxnelson]

Hi @onsi - you are right, yes, I am trying to test the actual upgrade to make sure that upgrading works. So I install an old version, do a few tests to make sure it is all good, then perform an upgrade, then the bulk of my tests are checking that the upgrade did what I expected it to do, and so I do need a little bit of order and I want those last batch of tests to be parallelized.

I was aware of the assumptions :) and I think that test independence is a great thing. We have a lot of tests we run, all with Ginkgo, and for almost all of them, those assumptions are totally fine. I just have a very small number of tests where I do need to do some things in a particular order, in order to test some feature. Upgrade is the prime example of that.

Our product can currently only have one instance in a k8s cluster. We run tests on ephemeral clusters though, and on different variants and versions of k8s.

The structure I outlined in the first post does actually work and do what I need, but I'm concerned that it will be difficult to maintain as people visiting the test code later might not have a good mental model of the sequence of events that I am kind of artificially constructing there, and it would be easy to make a mistake and invalidate the test.

[onsi]

Thanks @markxnelson - make sense. Given that what I'd personally do in a situation like this (and this may be more about subjective style/preference than anything else) is actually make a new test suite (called upgrade or something) and then have the BeforeSuite make the k8s cluster, install the old version, run the pre-check tests (inline) to make sure all is good (this is just part of the setup after all) then run the upgrade. Then I'd fill the suite with specs that make assertions, in parallel, on the upgraded code. Now folks visiting the code will know that the upgrade suite is entirely within the context of upgrades.

Alternatively, if your BeforeEach approach is working fine, then in lieu of a custom decorator I'd do something like:

var DescribeUpgraded = func(text string, body func()) {
  Describe(text, Offset(1),  func() {
      BeforeEach { wait until the upgrade in the other spec finishes }
      body()
  })
}

which would let you write

DescribeUpgraded("magic happens", func() {
  It {}
  It {}
})

I could imagine some sort of support for custom decorators that basically get a chance to inject arbitrary nodes into a container node. Though I'd need to think through the design and implications a bit more before building it out.

Last thought - one thing I'd worry about with the BeforeEach approach is the case where all the "RequiresUpgrade" nodes happen to run first and then block forever because the Describe that does the actual upgrading never runs. If you're running in parallel and randomizing the tests this situation will eventually happen... and be hard to debug.

[markxnelson]

Thanks, I will have to read up on Offset, have not used that yet. By the way the big doc update with v2 was really helpful!
I had thought about multiple suites, and I guess I could do that. That would mean I could not just do a ginkgo -p whatever/... though, but its not the end of the world, we have a lot of suites anyway, and they can't all be run like that because some are "safe" and some are "destructive."
I had noticed already the last issue you mentioned - if I run that without -p and you're unlucky, it will run the wrong one first and block forever.
I'll go away and do some more prototyping and let you know where I get to.
Thanks a lot for your help!

[onsi]

By the way the big doc update with v2 was really helpful!

❤️ thanks for the kind words!

we have a lot of suites anyway, and they can't all be run like that because some are "safe" and some are "destructive."

ooh... that's interesting. can you say more? Serial might help here because I often find that only a subset of specs are "unsafe". With Serial you can tell Ginkgo to run these in series at the end. That lets you run ginkgo -p ./... and not worry that some suites are safe for parallel while others are not.

Right now you have to mark individual specs/containers as Serial but I'm open to making it so that RunSpecs can be decorated to mark the entire suite as Serial. Let me know if that would be helpful to you.

[markxnelson]

Yeah, so by "safe" and "unsafe," I really mean whether they can share a cluster. For example, we have tests that test installing our product, upgrading our product, uninstalling our product. Those need to run in their own dedicated clusters because they are destructive.
Then I have a heap of tests that test the normal features of our product while its running - and most of those are "safe" and can be run in parallel, and can share clusters and so on.
I had not picked up that Serials all run at the end - that is interesting too!

[markxnelson]

I had a random crazy thought - and this is probably not a smart thing to do, but ...

What if I did something like make a func that takes a slice of It funcs and just runs them in their own goroutines... something like this

func Concurrently(assertions ...func()) {
	number := len(assertions)
	var wg sync.WaitGroup
	wg.Add(number)
	for _, assertion := range assertions {
		go assert(&wg, assertion)
	}
	wg.Wait()
}

func assert(wg *sync.WaitGroup, assertion func()) {
	defer wg.Done()
	defer ginkgo.GinkgoRecover()
	assertion()
}

but with the It blocks, not just regular funcs ("assertions" in this snippet).

I guess then I'd get some parallelism, but all in one process, and I assume it'd upset Ginkgo.

[onsi]

yeah that wouldn't play well. the Its all need to be invoked during the tree construction phase. I really do think a separate suite is your friend here.

And then possibly some investment in getting it so you can run ginkgo -p ./... safely. My sense is that will scale the most long-term without introducing a ton of complexity into your testing universe. I can imagine a shared library that can be used in the various BeforeSuites to either spin up a cluster if needed or pick a unique one from a pool (for those destructive tests) or attach to an existing shared cluster for those tests that can be shared.

[markxnelson]

Yeah I think I am leaning towards something like that - a shared library of funcs to do those kinds of things. :)

[dhiller]

ooh... that's interesting. can you say more? Serial might help here because I often find that only a subset of specs are "unsafe".

We currently have a scenario where we run all the parallel test cases (which gives us enough signal to understand what is wrong), and only run the serial ones if none of the parallel test cases has failed. We are now switching to using Serial, but can't find that ability. Is that possible?

[onsi]

hey @dhiller I don't think this is easily doable right now. A couple of options you can use today:

1. ginkgo --fail-fast will abort the suite after the first failure. Thought it sounds like you want to run all the parallel specs and gate the decision on running the serial specs on that outcome.
2. Use labels instead of the serial decorator and orchestrate things yourself in a script that invokes the test suite. Something like (in bash... ish. I haven't actually tried this to make sure I got it right):

ginkgo -p -label-filter="!serial"
if [ $? -eq 0 ]; then
  ginkgo -label-filter="serial"
fi

3. Set up some complicated thing in your suite where a ReportAfterEach reports on the state to a server you spin up in a SynchronizedBeforeSuite on the primary process and then Skip() the Serial specs depending on the aggregate state. (You could - but this will add a fair bit of complexity).

---

On the Ginkgo side of things I think the way I'd want to approach this is to add a new CurrentSuiteReport() that is analogous to CurrentSpecReport() that can get you an aggregated copy of the entire suite report on-demand. I'm not going to have time to get to this until after Thanksgiving break but I could see it making use cases like this much easier to implement.

[dhiller]

Thanks for your insights and the fast answer!

Yes, as you said, we don't want to use --fail-fast since we want to run the entire parallel test suite and only run the Serial ones if none of the parallel tests have failed.

Regarding 2.: Yes, this is what our current setup based on bash does, but we'd really like to get rid of that quite complicated bash code, use Serial decorator and leave it up to the Ginkgo test runner to handle it.

The reason for this is the problems we had with handling of multiple output files for Junit which we need to merge in a follow up step, and problems regarding duplication of test names and stuff. I'm currently working on this here, that's why I am asking.

I am going to split the PR into parts to have the Serial decorator and changes on Ginkgo side ready anyway, and keep the logic as is until until you have support for the CurrentSuiteReport(), which would ease the integration of that.

Thanks again and have a happy thanksgiving :)

[dhiller]

I had a thought that the generalization of this is something like a staged approach where you only want to run the next stage where the previous one has succeeded. But that of course is already present in CI systems like Jenkins, and not sure whether it makes sense to rebuild this for Ginkgo.

[dhiller]

On the Ginkgo side of things I think the way I'd want to approach this is to add a new CurrentSuiteReport() that is analogous to CurrentSpecReport() that can get you an aggregated copy of the entire suite report on-demand. I'm not going to have time to get to this until after Thanksgiving break but I could see it making use cases like this much easier to implement.

Hey @onsi! While I was revisiting the topic after quite a while I've been looking into the codebase and I haven't seen anything regarding CurrentSuiteReport() there.

Do you still have plans to implement this?

Thank you!

[onsi]

hey @dhiller - yes, still have plans but have struggled to invest as much time as I'd like in Ginkgo the last few months. I haven't lost sight of it though!

I'm also wanting to invest in better supporting the underlying usecase behind this thread: namely being able to better control parallelization and ordering of things in large complex integration suites. I have some ideas percolating and hope to have a draft proposal out.... eventually :/

[advdv]

I wanted to do this today, but not as away to change ordering but to inject common values into the context.Context that is passed to each test. In our case we want to insert idempotency keys, and request identifiers in almost every test.

And I think, in general, testing contextual stuff with Ginkgo often involves adding a lot of withValue lines at the start of a test (also for user sessions, middleware stuff, etc). A way to hook into Ginkgo unilaterally to inject these values might be desired. As an example, this is what we do now (notice the "IDed" wrappers we need to include everywhere):

var _ = Describe("user", func() {
	var mdl model.Model
	var tx pgx.Tx

	BeforeEach(tests.EnsureIsolatedTx(&tx))
	BeforeEach(tests.EnsureModel(fx.Populate(&mdl)))

	It("should setup model", tests.IDed(func(ctx context.Context) {

	}))

	It("should create and list users", tests.IDed(func(ctx context.Context) {

	}))

	It("should create and list organizations", tests.IDed(func(ctx context.Context) {

	}))
})

It would be nice if we could do something like this:

var _ = Describe("user", tests.IDed, func() {
	var mdl model.Model
	var tx pgx.Tx

	BeforeEach(tests.EnsureIsolatedTx(&tx))
	BeforeEach(tests.EnsureModel(fx.Populate(&mdl)))

	It("should setup model", func(ctx context.Context) {

	})

	It("should create and list users", func(ctx context.Context) {

	})

	It("should create and list organizations", func(ctx context.Context) {

	})
})

this is the "IDed" function we use now

// IDed that can wrap test functions to include identifiers that we use throughout the
// models and tests.
func IDed(fn func(context.Context)) func(ctx context.Context) {
	GinkgoHelper()

	return func(ctx context.Context) {
		ctx = some.WithRandomID1(ctx)
		ctx = some.WithRandomID2(ctx)
		ctx = some.WithRandomID2(ctx)

		fn(ctx)
	}
}

[onsi]

hey @advanderveer sorry for the delay - i've been out of pocket for a few weeks. A similar (not identical) set of questions was raised on this issue.

I could imagine a generic transformer decorator of type ginkgo.ContextTransformer with signature func (ctx context.Context) ctx.Context. Any runnable node that accepts a context (which would include setup nodes and cleanup nodes) would receive a context that is first passed through that decorator. I could imagine some edge cases and would want to make sure the current lifecycle of the context object makes sense with what you're proposing.

All in all your proposal sounds interesting to me. Do you mind opening a new issue so we can discuss it in more detail?

[advdv]

I could imagine a generic transformer decorator of type ginkgo.ContextTransformer with signature func (ctx context.Context) ctx.Context. Any runnable node that accepts a context (which would include setup nodes and cleanup nodes) would receive a context that is first passed through that decorator. I could imagine some edge cases and would want to make sure the current lifecycle of the context object makes sense with what you're proposing.

This sounds exciting, sorry for the delay in this, I've created the following issue: #1404