Add BeforeAll and AfterAll hooks

[aurelien-reeves]

Description

Adding BeforeAll and AfterAll hooks. They are executed before all scenarios are executed and
after all scenarios have been executed.

Like the javascript ones, they do not have access to the World and they cannot mutate the test environment. Their purpose is to set-up and/or clean-up external things like databases or browsers.

I choose to keep the AfterConfiguration hook as-is. We could consider its purpose not totally the same: AfterConfiguration has access to the configuration, and can even update it. BeforeAll cannot. This is still opened to discussion :p

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds new behaviour)
• This change requires an update of cucumber.io/docs

Update required of cucumber.io/docs

If the Cucumber documentation requires an update,
submit a PR to cucumber/docs and
reference it here.

e.g. "Ref: cucumber/docs/pull/#99"

Checklist:

Your PR is ready for review once the following checklist is
complete. You can also add some checks if you want to.

• Tests have been added for any changes to behaviour of the code
• New and existing tests are passing locally and on CI
• bundle exec rubocop reports no offenses
• RDoc comments have been updated
• CHANGELOG.md has been updated

[aurelien-reeves]

So, I choose not to share the World with BeforeAll and AfterAll hooks. It was easier to implement, cucumber-js also does that, and it prevents the users trying to share things using the World that would actually be lost because - if I am not wrong - the World is newly instanciated for each scenario.

But still, BeforeAll and AfterAll could be used for globally settin-up and cleaning-up things like database, browsers, and things related to the subject under test.

What do you think?

[aurelien-reeves]

Question: should we prevent invoking those hooks in dry-run?

[luke-hill]

Should these hooks have access to World

No they should not, it doesn't make sense and doesn't cause any issues

I have kept AfterConfiguration as well as BeforeAll

That's fine, but in my eyes why don't we just move the logic from AfterConfiguration to BeforeAll - maybe over 2 majors. The concept of anything with the word After in it being ran first is just plain wrong imo.

Should we prevent running hooks in dry run

Yes because dry-run should concern itself with things that a feature/scenario may or may not do (Not a suite). But I am not overly concerned with this

[aurelien-reeves]

Should these hooks have access to World

No they should not, it doesn't make sense and doesn't cause any issues

👍

I have kept AfterConfiguration as well as BeforeAll

That's fine, but in my eyes why don't we just move the logic from AfterConfiguration to BeforeAll - maybe over 2 majors. The concept of anything with the word After in it being ran first is just plain wrong imo.

So I would suggest to deprecate AfterConfiguration in favor of InstallPlugin rather than BeforeAll. InstallPlugin hook has the configuration object, BeforeAll has not.

Should we prevent running hooks in dry run

Yes because dry-run should concern itself with things that a feature/scenario may or may not do (Not a suite). But I am not overly concerned with this

👍
I'll see what I can do

[aurelien-reeves]

As part of this PR, I've added a short document to summarize all the hooks and their purpose.

Could you please review it? It is there: features/docs/writing_support_code/hooks/README.md

[luke-hill]

2 second look looks good for docs, I'll have plenty of review comments but it'll be minor polishes. I'll get to reviewing it in the next day or so. (if you can hold out till then that'll be good).

[aurelien-reeves]

2 second look looks good for docs, I'll have plenty of review comments but it'll be minor polishes. I'll get to reviewing it in the next day or so. (if you can hold out till then that'll be good).

No worries, no rush, take your time :)

[mattwynne]

Should these hooks have access to World

There is a use-case for these hooks being able to set some kind of read-only initial state/context so that, for example, you could access the database connection you created in the BeforeAll hook from step defs. There are obviously other ways of doing this in Ruby, but if we can see a way to do this and avoid people having to use class props / global variables, that would be nice.

For example, we could pass a reference to a mutable test_run_context hash into this hook, which we then surface as a read-only hash in the World hook.

Of course, we can do that as a second iteration.

[aurelien-reeves]

There is a use-case for these hooks being able to set some kind of read-only initial state/context so that, for example, you could access the database connection you created in the BeforeAll hook from step defs. There are obviously other ways of doing this in Ruby, but if we can see a way to do this and avoid people having to use class props / global variables, that would be nice.

For example, we could pass a reference to a mutable test_run_context hash into this hook, which we then surface as a read-only hash in the World hook.

Of course, we can do that as a second iteration.

As you said, I suggest to do that as part of a second iteration, or even just waiting for user feedback and requests