Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
alexkaufman06 opened this issue Jul 20, 2020 · 23 comments · Fixed by #1770
Closed

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

alexkaufman06 opened this issue Jul 20, 2020 · 23 comments · Fixed by #1770
Labels
⚡ enhancement Request for new functionality

Comments

@alexkaufman06
Copy link

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?

@davidjgoss
Copy link
Contributor

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

@charlierudolph
Copy link
Member

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

@alexkaufman06
Copy link
Author

@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!

@davidjgoss
Copy link
Contributor

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

@flywojt
Copy link

flywojt commented Jul 26, 2020

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

@alexkaufman06
Copy link
Author

alexkaufman06 commented Jul 29, 2020

@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
Copy link
Author

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

@davidjgoss
Copy link
Contributor

davidjgoss commented Jan 25, 2021

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?

@charlierudolph
Copy link
Member

@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?

@davidjgoss
Copy link
Contributor

davidjgoss commented Jan 31, 2021

@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.

@aslakhellesoy aslakhellesoy added the ⚡ enhancement Request for new functionality label Feb 2, 2021
BlueMona pushed a commit to BlueMona/cucumber-js that referenced this issue Aug 17, 2021
This is a draft failing scenario to try and describe what @charlierudolph and
@davidjgoss are describing in cucumber#1393

Feedback welcome.

Co-authored-by: Matt Wynne <matt@cucumber.io>
@mattwynne
Copy link
Member

mattwynne commented Aug 17, 2021

@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.

@lexevalton
Copy link

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 !

@davidjgoss
Copy link
Contributor

@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

@kamil1014
Copy link

Any updates?

@michael-lloyd-morris
Copy link
Contributor

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.

@davidjgoss
Copy link
Contributor

davidjgoss commented Mar 22, 2023

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.

@pk
Copy link

pk commented Jun 12, 2023

+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.

@aartajew
Copy link

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

@michael-lloyd-morris
Copy link
Contributor

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?

@Antoine-Regembal
Copy link

Also interested for this improvement 👍

@michael-lloyd-morris
Copy link
Contributor

michael-lloyd-morris commented Sep 1, 2023

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.

@mattwynne
Copy link
Member

NB that the PR #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.

@davidjgoss
Copy link
Contributor

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

See the documentation linked from the changelog entry for usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚡ enhancement Request for new functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.