Doco / ThreadLocal State

[Johnlon]

🤔 What's changed?

Advice on state on the JVM.

⚡️ What's your motivation?

Avoid accidents.

📋 Checklist:

• [x] I agree to respect and uphold the Cucumber Community Code of Conduct

[mpkorstanje]

The aim of this bit of documentation is to inform the user not to share state between scenarios. Scenarios are independent units of execution. As such advice on how to share it should not be provided.

Advice on how to rework you scenarios to be independent would be useful but would be somewhat involved and might be better suited to a blog format.

[Johnlon]

I understand the point about the advice being given in the scenario section. Having read that page many times with others I don't think the page communicates that well. I'll have a go at rewording to be more direct.

Re my advice. Shared state between steps on the JVM is unsafe in the presence of MT without the advice I've given. We know that scenarios may be run in parallel depending on the test framework used or depending on whether the CLI is used with the -thread option. But we also know that steps in a scenario are always run sequentially on a single thread. We also expect certain steps to be common between scenarios. This sharing of steps means state sharing is a risk without the use of a "thread local" mechanism.

As a developer of features on the JVM the advice I've written is the advice I'd have liked to have been given out of the box, but instead I had to discover the hard way. I believe it's on us to give the users a nudge in the right direction rather than letting them learn by having them suffer tricky issues. The best place for such advice is the location that users will be most likely to discover it naturally. So I think the state docs for the jvm are the appropriate place.

Like I say, I'll see if I can improve on the direction of that page.

[mpkorstanje]

Shared state between steps on the JVM is unsafe in the presence of MT without the advice I've given.

I'm afraid you are mistaken. I suppose you may have missed the dependency injection section of the documentation and are now dependent on static variables.

https://docs.cucumber.io/docs/cucumber/state/#dependency-injection

Unfortunately I don't have the time to debug your current mental model of Cucumber through repeated questions and answers. And from interactions elsewhere you seem to be unwilling to reconsider your assumptions in any other fashion.

[mpkorstanje]

I have not reviewed any subsequent edits. Would advise other reviewers to strongly consider the necessity.

[Johnlon]

@mpkorstanje

I realise now why I got confused re thread locals.....

--

I've removed the threadlocal comment because it's not needed on step classes (alone) due to new step instances being created for each scenario.

--

However Spring seems to have a few risks in this space.

My experiences and experiments indicate that a single instance of the app context is used by all scenarios. Therefore any objects in the app ctx are effectively "global" state and are a vector for one scenario to interfere with another. In my own code I had injected a single "state" object to all the step classes and I had observed the spring app ctx was a singleton across all scenarios and changed this state class use a thread local internally to stop the risk of scenarios from trampling each other.

So, I have added a word of caution about the use of spring for state sharing as this appears to be a hazard.

Looking at this statement I got the impression that it was safe to share state via spring ... "dependency injection (DI) module to organize your code better and to share state between step definitions".

I have added a few words of caution on spring. I do not know if the other DI frameworks are similarly afflicted.

Are your own experiences different re spring? Am I wrong in this?

[Johnlon]

@mpkorstanje apologies if I seem to have offended you. I am not sure why you've come to the conclusion that I am unwilling to take onboard info - I'm just trying to understand the constraints in the product better and improve it. As you'll see above I've evolved a bit.

[mpkorstanje]

Given what would be an obvious defect have you considered the possibility you may have missed some information?

For example the section about Spring in dependency injection links to GitHub. Specifically you may have missed:

https://github.com/cucumber/cucumber-jvm/tree/main/spring#sharing-state-between-steps

This is also an example of a step in a process that is a little too time consuming.

We started out with a request for feature hooks and have now worked our way down to a missed piece of documentation. You appear to be an otherwise capable engineer, I would have expected you to be make this leap on your own.

Now assuming that this gets us to the root cause of the problem I'm happy to accept contributions that improve this aspect of the documentation. But you will have to look at how someone (not specifically you) might find, interpret and understand the docs and where they'd go wrong.

It would also help if you could lead with this information as if it was a bug report. E.g. I misunderstood Q to mean R because S while T should have been implied.

You may also find reading about the XY problem to be informative: https://xyproblem.info/

[Johnlon]

@mpkorstanje Thanks I totally missed that. BTW I wasn't the only person who missed that annotation doco - so I hope you'll accept the small changes that remain in this PR.

I've made two points clearer in the doco.

1. that shared state between scenarios is bad but steps are ok
   and
2. the spring repo's readme contains some important into on a thing called ScenarioScope.

I also put a block around the mention of the "World" so te title only shows up on the non-JVM views where it's applicable. I found the point that JVM doesn't support a "World" context as confusing and distracting. I can undo if you want.

https://deploy-preview-730--cucumber.netlify.app/docs/cucumber/state/

[mpkorstanje]

Okay finally merged. I forgot this PR existed.