Skip to content

Make Cucumber runner extend BlockJUnit4ClassRunner #1598

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

Closed
wants to merge 4 commits into from
Closed

Make Cucumber runner extend BlockJUnit4ClassRunner #1598

wants to merge 4 commits into from

Conversation

nicerloop
Copy link

Summary

This is a Proof-of-Concept to improve cucumber-jam integration with junit 4 by deriving Cucumber runner from BlockJUnit4ClassRunner in order to benefit from natural integration with unit 4 features like scenario rules (#393) and natural integration with external frameworks (http://pitest.org, https://www.testcontainers.org, https://docs.spring.io/spring/docs/5.1.6.RELEASE/spring-framework-reference/testing.html#testcontext-junit4-rules)

Details

The change proven here is to derive Cucumber unit runner from BlockJUnit4ClassRunner rather than from ParentRunner, which requires in this version the make the FeatureRunner disappear to directly host scenarios/Pickles at the same level as scenarios/methods in classical unit tests.

The current implementation is really a quick and dirty proof that is can work.

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • [?] New feature (non-breaking change which adds functionality).
  • [?] Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Apr 4, 2019

I'm impressed by what you've managed to do.

It is unfortunately quite hacky. I mean that in the sense the concepts involved are misaligned. The BlockJUnit4ClassRunner makes several assumptions that are not true for Cucumber. To wit it assume that a single method is a single test and that each method is executed on a test context which consists of a single class.

As a result of this we run into quite a few complications that would make JUnit rules work quite differently from how they'd normally would be expected to work. To wit a brief list of the problem areas:

  1. Steps do not have access to the rule instance. It's part of the JUnit test context which is not accessible from Cucumber. This is a requirement for most JUnit rules to work properly (e.g. TemporaryFolder).

  2. JUnit rules assume that any exceptions thrown by the method under test bubble up through the hierarchy of Statements where it can be caught (e.g. ExpectedException). This is again not true for Cucumber. All exceptions thrown in a step will be caught by the runner.

  3. Cucumber is generally expected to be executable by different runners. Allowing on runner to do more things then others would results in quite some unexpected situations. For instance running with The IDEA Cucumber Plugin (which uses the CLI) will have different results then when using JUnit. This is unfortunately already the case with @BeforeClass and @AfterClass and as such their use is discouraged.

So instead of trying to push all the square pegs into round holes I think it would be better to let the user deal with it. For most cases they an do so by initializing the JUnit rule in a step definition and invoke the rules before and after methods by using Cucumbers before and after hooks. It's not quite perfect but it keeps the ambiguity out of the framework.

One exception here is Spring. We currently do support Spring through the cucumber-spring module. Unfortunately this is support is less then perfect. Our SpringObjectFactory does not use the TestContextManager provided by Spring. As a results not all annotations work. There is an open issues (#1470) to resolve this if you'd be interested in doing so.

That said. I would like to be able to somewhat properly integrate JUnit Rules with Cucumber. To do so however we (much like the JUnit 5 project) would need to make our test context an explicit concept in the framework. This would require a non-trivial engineering effort. You would be most welcome to have a (collaborative) stab at it. If you would be interested in this please do hop onto the cucumber slack to discuss this.

@coveralls
Copy link

coveralls commented Apr 5, 2019

Coverage Status

Coverage decreased (-0.2%) to 86.0% when pulling 8d07b4b on nicerloop:master into 6596761 on cucumber:master.

@nicerloop
Copy link
Author

I was well aware that it is really a hack, just to confirm it is doable. After discussions with my colleagues, I concluded the appropriate mapping between cucumber and junit4 is as follows: Cucumber / Suite, FeatureRunner / BlockJUnit4ClassRunner, PickleRunner / FrameworkMethod.

I implemented this mapping, which requires completely removing the step description feature. This does not seem to be that problematic, as it has been identified to be removed in #263 (comment) .

As for the problems you identified, they are real but:

  1. junit context (test class) not directly accessible from the cucumber steps, is the same problem as cucumber state management, which can be worked-around by a world object or dependency injection

  2. exceptions caught by the runner is not a problem for me for now, as I don't want to use rules for scenario control, but rather for external state setup and teardown in integration tests: temporary folder, database docker container, spring test rules...

  3. cucumber may be executed by different runners, but the junit ecosystem is much larger than the cucumber one, and I want to reuse all those junit extensions modelled as rules to pimp my cucumber tests, rather than write and maintain integrations by copy-pasting what is already done, for no effective gain; junit rules and class rules complement cucumber hooks, as @around and global hooks are not available

Just have a look at the pitest extension for cucumber https://github.com/alexvictoor/pitest-cucumber-plugin which contains much copy-paste to upgrade dependencies versions, whereas this improved integration between cucumber and junit brings directly the integration with pitest for no additional cost.

As for using rules from the before and after hooks, not all rules allow that, as the rule design is to decorate statements. In fact, it is not possible with the spring rules. And as the cucumber-spring modules does not provide integration with the testcontextmanager, using junit rules as designed is the easiest way for me.

I really hope to be able to contribute this evolution, as it effectively allows my teams to use cucumber with spring applications, mutation testing and docker containers.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Apr 6, 2019

As it stands in v4 the dependency injection context is an implementation detail of the JavaBackend. You may be able to apply a decorator pattern with v5 (see the develop-v5 branch) but I imagine it would be quite clunky.

I don't find your cost argument to be particularly convincing. Your proposed changes would incur additional complexity to Cucumber to allow for functionality that has a limited application. Additionally because this functionality doesn't quite work as expected it will be a constant source of tickets, questions, confusion and complaints. It then seems only appropriate that both the cost and benefit of this functionality are in the same place.

So given that you are unable to contribute to an improved spring module or an explicit test context I I would recommend that for the time being you maintain and publish your own runner. We can make this easier by reducing the amount of boiler plate needed though! Comparing Runtime with Cucumber, TestNGCucumberRunner and CucumberTestUnitFinder I think there should be some options.

@lock
Copy link

lock bot commented May 20, 2020

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.

1 similar comment
@lock
Copy link

lock bot commented May 20, 2020

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 May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants