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

Before and After methods invoked for unused step definition classes #1005

Closed
jwgmeligmeyling opened this issue May 16, 2016 · 10 comments
Closed
Labels
⚡ enhancement Request for new functionality

Comments

@jwgmeligmeyling
Copy link

jwgmeligmeyling commented May 16, 2016

Currently, all classes that contain step definitions are instantiated and all before and after methods are ran. This behaviour is very unpreferable. For some stories you just don't need some (potentially heavy) domains to be setup. So if one scenario requires a web container to be set up, and an other scenario does not use any step definitions from that WebContainerStepDefinitions class, the @Before starting the web container should not be started for the second scenario.

class WebContainerStepDefinitions {
   @Before
   public void setUp {
         Thread.sleep(2000);
   }

   @Given("^Given the webserver has started$") {

   }
}

class OtherStepDefinitions {
   // Other step definitions, completely separated from WebContainerStepDefinitions 
}

Users are now working around this issue by introducing annotations/labels. This is in my opinion very unwanted, as your user stories now become aware of and dependent on the implementation of your step definitions. Furthermore it is error prone, because you now have step definitions available for use, for which state may not be initialized nor will be properly cleaned up, would you forget to use the correct annotation.

In my opinion the only clean solution is to only invoke @After and @Before methods from a class, if the scenario actually uses a step definition from that class.

See also the little debate we had at our course: SERG-Delft/jpacman-framework#92
This issue was also mentioned in issue: #993
Ping @RichardBradley

@brasmusson
Copy link
Contributor

Currently, all classes that contain step definitions are instantiated

No, classes that contain step definitions are only instantiated right before the (first) step definition or hook method defined in them is called.

all before and after methods are ran

Yes, the semantic of a @Before method is that it is executed once before each scenario. This can be restricted by the use of tags, so that the @Before method is only executed once before each scenario with the specified tags.

Conceptually both step definitions and hooks are global (if they are on the classpath step definitions can be used, and hooks will be executed), the class they are defined in a secondary, so there is really no concept for speaking about "the hooks of the classes from which step definitions are actually used".

@RichardBradley
Copy link
Contributor

Currently, all classes that contain step definitions are instantiated

No, classes that contain step definitions are only instantiated right before the (first) step definition or hook method defined in them is called.

I have also found that all step definition objects are instantiated for each scenario, even if they do not contain any steps which are used in that scenario.

Perhaps this behaviour is specific to the Java 8 steps (whose steps are defined at runtime, not statically) or perhaps it depends on which CI you use (I am using Picocontainer).

Conceptually both step definitions and hooks are global

I tend to agree with this, although I can see that @brasmusson's suggestion might be useful in practice.
Doing otherwise would require a certain amount of extra complexity in the runner's semantics, and there is a messy edge case where a Scenario is aborted due to error before reaching the first step from a given definition class -- does that count as a Scenario containing steps from that class or not?

I think the practical solution is to make the @before and @after methods very cheap to run if they are not needed (e.g. detect if any cleanup is needed and do nothing in @after if not).

@RichardBradley
Copy link
Contributor

re test classes from project dependencies: those classes should not be on your classpath: projects should not, by convention, export their test classes. See #999

@RichardBradley
Copy link
Contributor

Or at least have that controllability.

You may well be able to get fine-grained control of this if you switch the DI framework for the steps -- have you tried that?

@brasmusson
Copy link
Contributor

I challenge the statement "before and after methods for step definition classes", there is nothing that says that a class that define a before or after method also needs to defined a step definition. Comparing with an example in the Cucumber-Ruby project (examples/i18n/en), the step definitions are defined in the file features/step_definitions/calculator_steps.rb and the hooks are defined in the file features/support/hooks.rb, in Java terms that would mean that the hooks are defined in a different package from the step definitions.

The biggest problem with changing the behaviour "to only invoke @after and @before methods from a class, if the scenario actually uses a step definition from that class", is that since there are no requirement that a class that defines a hook also needs to define a step definition, the only safe assumption is that "in the wild" there are project which defines hooks in classes without step definitions. With this behaviour change, those projects would suddenly start to fail, and there may not be an easy way to fix that, the thinking the "before and after methods for step definitions classes" may not fit those project at all.

No, classes that contain step definitions are only instantiated right before the (first) step definition or hook method defined in them is called.

I have also found that all step definition objects are instantiated for each scenario, even if they do not contain any steps which are used in that scenario.

I ran the java-calculator with the three step definition classes instrumented so that the constructors print the class name, the results confirms that step definition classes are only instantiated when a hook or a step definition from them are called.

Or at least have that controllability.

I was not correct when I wrote "if they are on the classpath step definitions can be used, and hooks will be executed", the correct statement is "if they are on the classpath (of course), and they are in a package that Cucumber-JVM is instructed to search for hooks and step definitions (the --glue option)". The two way to control which hooks are executed are to carefully choose in which packages they are defined, and to specify tags for them.

The first thing I think of to solve the problem with the step definition class in the problem description if tagged hooks does not fit the problem, is not to use a before hook at all, but either to call setUp from the constructor, or to implement lazy initialization:

class WebContainerStepDefinitions {
   private boolean initialized;

   public void setUp {
         initialized = true;
         Thread.sleep(2000);
   }

   @Given("^Given the webserver has started$") {
         if (!initialize) {
            setUp();
         }
      ...
   }
}

@RichardBradley
Copy link
Contributor

re whether definition classes are instantiated even if not needed -- I expect this is a "Java8" difference. The java-calculator example uses definition classes with statically annotated Steps, so it is possible to determine if a class contains steps without instantiating it.

When Java8 step definitions are used, the steps are defined dynamically at runtime, see https://cucumber.io/docs/reference/jvm#lambda-expressions-java-8 . So cucumber won't be able to tell if a class contains any Steps or any relevant Steps without instantiating it.

Certainly, it is my experience that cucumber-jvm instantiates all my step definition classes for all my Scenarios even if none of the steps are used in that Scenario.

I don't think we need to settle this as part of this issue though -- it seems tangential to me.

@jwgmeligmeyling
Copy link
Author

jwgmeligmeyling commented May 19, 2016

I see how the @Before and @After are intended now, and why they are always invoked, and I also see the issues related to not running these hooks only if a step definition is used from that class as well. You should therefore keep the computation in these hooks relatively easy, potentially reusing the result from a previous run. However, sometimes you just have to setup more intensive context for some domains within (some of your) tests.

Currently there are two options:

  • Use tags. This requires you to make your stories aware of the implementation of the step definitions and classes, which in my opinion contradicts with the key benefit of writing user stories in the first place.
  • Separate features and their step definitions in separate packages, and run the features with a different glue package. This obstructs step definition reuse, because you now organise step definitions by feature, and not domain.

I am really looking for a mechanism that allows me to organise my step definitions by domain and be able to hook on scenarios only if this domain is used. My question is, can we do a better job? And how could we implement this (considering that my current idea - only running the hooks if a step definition from that class is used - is pretty much rejected).

re test classes from project dependencies: those classes should not be on your classpath: projects should not, by convention, export their test classes. See #999

That is another issue not related to this topic 😉 Got that!

@jwgmeligmeyling
Copy link
Author

Actually now I think of it, if the domains are properly organised by packages, and the step definitions for that domain share the same package, you can just load multiple glue packages. So that seems like the preferred way of doing this then.

@aslakhellesoy aslakhellesoy added Java ⚡ enhancement Request for new functionality labels Aug 30, 2016
@mpkorstanje
Copy link
Contributor

It seems this issue has reached a satisfying conclusion.

PS: Sup SERG.

@lock
Copy link

lock bot commented Oct 25, 2018

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.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⚡ enhancement Request for new functionality
Projects
None yet
Development

No branches or pull requests

5 participants