cucumber-js icon indicating copy to clipboard operation
cucumber-js copied to clipboard

Feature Request: Give BeforeAll a reference to the Cucumber World object

Open alexkaufman06 opened this issue 4 years ago • 14 comments

To get around this we've been rolling our own DI which is cumbersome. If this is by intention, I'm wondering why BeforeAll is not given this reference?

alexkaufman06 avatar Jul 20 '20 23:07 alexkaufman06

Hi @alexkaufman06

The problem with giving World to BeforeAll comes from these two things:

  • BeforeAll runs prior to, and independently of, all of the scenarios
  • Each scenario when run gets its own dedicated instance of a World

So what World can we reasonably provide to BeforeAll? Maybe its own instance, but that would lead to a lot of unexpected behaviour as people wonder a) why their World is initialised more times than they have scenarios and/or b) why state they set in the hook is not retained into the scenarios.

I've had some frustration with this myself in the past; what I found was I mostly wanted the world parameters in order to get at things like URLs and API keys so I could do some preflight tasks like switching on feature toggles etc. Could you expand on your use case a little?

cc @charlierudolph

davidjgoss avatar Jul 21 '20 07:07 davidjgoss

Thinking on this idea I think we could do something like the following:

  • this in every BeforeAll / AfterAll hook is a single testRunContext (that starts as an empty object)
  • World constructors are then passed the testRunContext as a parameter
  • For testRunContext to work in parallel mode, we need testRunContext to be json serializable

charlierudolph avatar Jul 21 '20 15:07 charlierudolph

@davidjgoss One use case is simply holding onto a reference for items that are ripe for reuse as well as gaining more autonomy in the execution flow. Specifically, it's things like storing access tokens and ensuring connections to faked downstream components that we're simulating are in place before we make any assertions.

@charlierudolph I like that idea!

alexkaufman06 avatar Jul 21 '20 17:07 alexkaufman06

@charlierudolph i like that. I think (maybe selfishly) it would be useful to have the test run context initialised with the world parameters too.

davidjgoss avatar Jul 21 '20 20:07 davidjgoss

@alexkaufman06 could you send code sample how you solve this problem? I have similar problem with AfterAll hook

flywojt avatar Jul 26 '20 12:07 flywojt

@flywojt there are multiple files that make this happen in my repo. The real magic comes from using Inversify. Hopefully that helps or we can a get better solution like mentioned above. I can try and share more info later if needed.

alexkaufman06 avatar Jul 29 '20 02:07 alexkaufman06

@davidjgoss @charlierudolph checking in on this issue. The solution Charlie mentioned seemed practical and helpful.

alexkaufman06 avatar Sep 14 '20 14:09 alexkaufman06

Another vote for BeforeAll and AfterAll having world parameters via #1547. The use case for getting URLs etc from world parameters in order to do setup seems reasonable (I could use this too).

@charlierudolph returning to your proposal above, only thing that gives me pause is that it could be an easy way for users to start having scenarios depend on each other by sharing state in that test run context. What do you think about passing in a read-only clone of the context to each world, so in effect whatever setup is done in hooks is then immutable?

davidjgoss avatar Jan 25 '21 10:01 davidjgoss

@davidjgoss yeah it would become a way to share state but there's only so much guidance we can give (and users can work around it if they want to anyway. Thus not sure we need to give much protection against doing it other than suggesting it in the docs. How are you thinking of creating something immutable?

charlierudolph avatar Jan 31 '21 00:01 charlierudolph

@charlierudolph could type as DeepReadonly with TypeScript (https://github.com/jbpros/cucumber-fp does this) which coupled with _.deepClone would go a long way.

To enforce at runtime could look at a recursive Object.freeze(), or maybe a proxy, could be bug-prone though.

Agree determined users will find a way.

davidjgoss avatar Jan 31 '21 09:01 davidjgoss

@BlueMona and I paired on a failing scenario for this today. I suggest we move the discussion over there and help her get the tests into shape and work towards an implementation.

mattwynne avatar Aug 17 '21 19:08 mattwynne

Hi @davidjgoss I have a use case for a world in BeforeAll -> As a QA i'm testing my app on different client instances. Not the same URL, not the same users registered but tests are the same. In my project, my BeforeAll hook open a "Users.json" to load each users that can be used in the tests. Since we cannot send to the BeforeAl hook on which client we are running our tests, for the moment we are renaming manually a client user file into the generic one. It doesn't allow us to inetgrate our tests in CI. (so we rename "Client_Users.json" into "Users.json" when we want to use this file). There are others basic settings I do in my BeforeAll hook based on which instance (mysql or MSSQL ? Client A or Client B?) I'm running my tests and being able to send these info to my hook would be perfect.

What I will do in the mean time is developing a powershell script that will take my client in argument (example "BananaClient", and it will rename "BananaClient_Users.json" into "Users.json" to let my BeforeAll hook open the Users.json. It's the only way I found to have multiple clients configurations files without doing myself renaming of files.

In fact, I don't know for others but, I never used parameters I receive in the world of in my scenarios. What I send in argument of my command line is for the setup of my test instance, not for my scenarios. I don't know if there a way to say "this parameters of my command line is for beforeAll and others are for all scenarios" but it would be a good compromise perhaps ?

Thanks !

lexevalton avatar May 18 '22 20:05 lexevalton

@lexevalton re the world parameters, the idea we're going with for BeforeAll is that:

  • It'll have access to the world parameters (just as the world does)
  • It'll be able to set some global state that will be readable by the world

davidjgoss avatar May 24 '22 08:05 davidjgoss

Any updates?

kamil1014 avatar Sep 12 '22 10:09 kamil1014

What about allowing BeforeAll to preprocess the world parameters? If it returns an object, that object will be the new world parameters - otherwise the world parameters remain unchanged.

Whatever is decided on, I'm interested in coding it up.

michael-lloyd-morris avatar Mar 20 '23 23:03 michael-lloyd-morris

What about allowing BeforeAll to preprocess the world parameters? If it returns an object, that object will be the new world parameters - otherwise the world parameters remain unchanged.

Interesting idea. It would get away from having to have a new "thing" called test run context if we are just accessing and updating the world parameters. And we did say it has to be serialisable anyway. @BlueMona and @mattwynne you've been working on that PR, what do you think on this? Probably something I'm not thinking of.

davidjgoss avatar Mar 22 '23 11:03 davidjgoss

+1 for having either World instance or at least the processed parameters available in the BeforeAll. Our use case is that we run test suites in various configurations (locally & CI) and I'd like to:

  1. Log configuration to console ENV variables gets processed in cucumber.mjs, worldParams are constructed, specific config profile is run, so I can't dump it into console in the cucumber.mjs

  2. Attach configuration into a report (in the future we will have custom report)

I could workaround this by creating a Feature file setup-test-suite.feature (and running it first) where I could implement steps to dump configuration and attach it but... it would be better to have this done in the BeforeAll.

pk avatar Jun 12 '23 11:06 pk

As a workaround, you can load Cucumber configuration (together with worldParameters) outside of steps using API:

import { loadConfiguration } from '@cucumber/cucumber/api'
import ArgvParser from '@cucumber/cucumber/lib/configuration/argv_parser'

// store original Cucumber argv (needed for parallel runs which spawns child processes with stripped argv)
if (!process.env.CUCUMBER_ARGV) {
  process.env.CUCUMBER_ARGV = process.argv.join(' ')
}

export async function loadWorldParams() {
  const argv = process.env.CUCUMBER_ARGV!.split(' ')
  const { options, configuration } = ArgvParser.parse(argv)
  const { useConfiguration } = await loadConfiguration({
    file: options.config,
    profiles: options.profile,
    provided: configuration,
  })

  return useConfiguration.worldParameters
}

Works on Cucumber v9.2.0

aartajew avatar Jul 16 '23 17:07 aartajew

Workarounds are great, but even better are improvements to the engine.

@mattwynne and @BlueMona - Is there anything in your project that conflicts with what I propose?

michael-lloyd-morris avatar Jul 16 '23 21:07 michael-lloyd-morris

Also interested for this improvement 👍

Antoine-Regembal avatar Sep 01 '23 14:09 Antoine-Regembal

I'm going to begin work on this next week. I presume that this will need to be version 10 because if someone does a return from a BeforeAll (why they should I have no idea) it is bc break. Also, as a precaution I need to make sure BeforeAll hooks resolve sequentially so that all their changes get applied. The order should not be important, but if someone needs the beforeAll hooks to resolve in the same order that would also be a break.

Also, we might wish to consider having a hook that specifically does this behavior and leave BeforeAll alone. In otherwise, a Setup hook. If that's the route chosen I would advise specifying the following:

  1. Only one Setup hook allowed - trying to declare a second one pitches an error
  2. Setup MUST return worldParameters

Either approach is fine by me, but I'd like input as to which approach to take.

michael-lloyd-morris avatar Sep 01 '23 19:09 michael-lloyd-morris

NB that the PR https://github.com/cucumber/cucumber-js/pull/1770 is very close to complete, I was supporting @BlueMona with it but I'm not sure if she has capacity / inclination to finish it, perhaps someone else would like to take that on.

mattwynne avatar Sep 01 '23 23:09 mattwynne

Released in https://github.com/cucumber/cucumber-js/releases/tag/v10.1.0

See the documentation linked from the changelog entry for usage.

davidjgoss avatar Dec 21 '23 17:12 davidjgoss