support code library: interface change

[charlierudolph]

Currently for any support file, it if exports a function, the function is called. This is not ideal because in javascript classes are functions and if you have a support file that exports a class it will be run. This is not desired and will sometimes error due to the constructor expecting parameters or if the class verifies it is only called with new.

@aslakhellesoy @mattwynne can you explain how your cucumber versions load support files and plusses / minuses you have seen so far?

[charlierudolph]

@jbpros I'd like to get your thoughts as well

[jbpros]

@charlierudolph I'm well aware of this constraint and definitely open to revisiting the interface.

Do you have ideas already?

Something like the following would be great:

import { Given, When, Then, Before, After, defineWorld, defineStep, registerHandler } from 'cucumber'

class MyWorld {}

defineWorld(MyWorld)
Before(() => ...
Given("something", ...

It makes things a little bit more complicated in browser-land. Think about the in-browser example: with the current interface, it's easy to make a new instance of cucumber for each run within the webpage and have the (potentially modified) support code reevaluated and passed to it. The API I suggest above would need some support code "reset" between each cucumber runs as everything defined in it would be "global" and not encapsulated into some re-runnable function.

WDYT?

[charlierudolph]

If we are moving to globals I would rather just expose the functions as globals so they don't need to be imported (similar to what mocha does with describe / it).

The other idea I've thought about was that the exported function name needs to match something or have some other property to differentiate it. Haven't come up with something I like around this though

[jbpros]

Well, technically, I'm not suggesting anything global per se. What I'm talking about is a singleton that holds the state related to step definitions and hooks. And that singleton could be a "default instance", we could expose a proper instance-based API like so:

import { defineSupportCode, Before, Given } from 'cucumber'

const supportCode = defineSupportCode(() => {
  Before(...
  Given(...
  // defineSupportCode could even expose the usual helpers to avoid explicit imports, similar to the current API:
  this.When(...
})

And calling the helpers directly as suggested previously would in fact call defineSupportCode behind the scenes and keep a reference to that single instance that would be used by default by Cucumber.

This would give us the advantages of both approaches:

• no more constructor automatically called
• no need to call this.xxx all over the place
• still able to programmatically pass isolated support code to Cucumber (useful in an environment that do multiple runs, like a browser)
• no globals

I'm against introducing mocha-like global helpers. In mocha, it's (kinda) OK because those tests are meant to be quite isolated, focussing on small(er) units. Cucumber, on the other hand, is used for acceptance testing. That often means loading a lot of things and polluting the global scope is more dangerous in that context. Adding import { Given, When, Then } from 'cucumber' at the top of your files to keep things clean and safe is really worth it, imho.

[charlierudolph]

I don't like requiring it from cucumber as I don't see an easy way to hook that up with the running instance of cucumber (though I'm all ears if you have an idea on how to do that).

I do like the idea of defineSupportCode though. That can be the single global exposed and it exposes the helpers as you suggested.

defineSupportCode({After, Before, Given, When, Then}) => {
  // ...
})

When cucumber loads a support file it can assign the global just in time and remove it after loading.

[jbpros]

I don't like requiring it from cucumber as I don't see an easy way to hook that up with the running instance of cucumber (though I'm all ears if you have an idea on how to do that).

One way would be to have a support code library instance attached to the Runtime class (i.e. a static property). import { Given, When, Then } from 'cucumber' would expose the helpers from that instance. Then the runtime would implicitly use that static support code library instance, if no explicit one is passed to its constructor.

In most cases, that's what people would do.

Now, let's consider the browser context: as the process does not end like it does on the CLI, we might need to reset the support code library between runs. I see two solutions:

1. Simply replace the static support code library with an new instance (Runtime.resetSupportCodeLibrary()?) so that the next runtime instance can pick it up.
2. Use the defineSupportCode(({After, Before, Given, When, Then}) => {...}) approach: the client would create a SupportCodeLibrary instance itself and pass it explicitly to the Runtime when instantiating it.

And actually, having both 1 & 2 available would make sense. The former is simple but does not allow advanced things like multiple runtimes running in parallel, the latter is a bit more complicated but completely flexible and fully isolated.

[charlierudolph]

Available in 2.0.0-rc.4.

[nicojs]

Kudos implementing this issue! The exporting functions is what kept me on the fence so far.

@charlierudolph

When cucumber loads a support file it can assign the global just in time and remove it after loading.

Why not importing Given, When, etc directly from 'cucumber' as @jbpros suggested? Can i try to implement this? I think it is even more clean.

[nicojs]

@charlierudolph any updates on this? Or should i open a new issue?

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.