Feature: keep some blank lines when many cases are part of a single scenario

[Jiehong]

🤔 What's the problem you're trying to solve?

Sometimes, a feature file contains 1 scenario, but this scenario has multiple "given, when then" parts, each separated with a blank line and comment.

Today, gherkin-utils removes those empty lines, and makes everything compact. But, in this case, this is worse.

Original (and expected):

scenario:
  # First case
  Given path 'path1'
  And request myRequest
  When method post
  Then status 201
  * def myId = response.data.id

  # Second case
  Given path 'path1', myId
  When method get
  Then status 200

  # Third case
  Given 'path1', myId
  When method delete
  Then status 204

After formatting:

scenario:
  # First case
  Given path 'path1'
  And request myRequest
  When method post
  Then status 201
  * def myId = response.data.id
  # Second case
  Given path 'path1', myId
  When method get
  Then status 200
  # Third case
  Given 'path1', myId
  When method delete
  Then status 204

(in reality, we can have cases with 8 or 9 cases in a scenario, and having those blanks lines is really helpful).

✨ What's your proposed solution?

Always keep a blank line before a line containing only a comment, unless it directly follows a scenario step.

⛏ Have you considered any alternatives or workarounds?

Using multiple scenarios is not always possible in this case, because those cases are linked, and use data from each other.

📚 Any additional context?

Version used of gherkin-utils: v8.0.5, the java version via the maven spotless plugin.

[kieran-ryan]

Hi @Jiehong, a nice issue for sure! For each of your cases, is it also possible to rewrite them as below; with a scenario for each case?

I believe the gherkin specification and parser are designed to have separate scenarios for each 'case'. Do other gherkin tools support this format for example, or are there any other gherkin tools that don't support this format? Backgrounds can also be leveraged to prepend precondition test steps where needed. What do you think?

Scenario:
  Given path 'path1'
  And request myRequest
  When method post
  Then status 201
  * def myId = response.data.id

Scenario:
  Given path 'path1', myId
  When method get
  Then status 200

Scenario:
  Given 'path1', myId
  When method delete
  Then status 204

[Jiehong]

We're actually using karate (https://github.com/karatelabs/karate), and the first screenshot is about this case, actually:

https://github.com/karatelabs/karate/raw/master/karate-demo/src/test/resources/karate-hello-world.jpg

One of the interesting part of karate, is that it can run everything in parallel: features but also scenarios (https://github.com/karatelabs/karate#parallel-stats).

This means that those 3 cases cannot be run in parallel, and therefore have to be part of the same scenarios (since they all depend on the first call, requiring the id to execute the second and third case).

Perhaps having a blank line before a Given: line could be enough?

[mpkorstanje]

Based on this specific usecase I don't see a good reason to support this. This would solve a problem caused by another problem in a rather adhoc fashion. And any solution that works for this case may have unintended consequences down the line for other use cases. I reckon the complexity and time spent dealing with this would be a bit much.

If you can not make your scenarios independent, as a work around I would suggest adding an second empty comment line to add some more visual space.

But I would strongly suggest trying to make your scenarios independent first. One way to do this would be to have each scenario create a unique resource rather then reuse it between scenarios. And instead of having a scenario for clean up, you would do the cleanup in the after hooks.

[Jiehong]

In my case, the scenario is to test 3 http endpoints: POST, GET and DELETE.

So it's a bit tricky to test the cleanup as an after hook, or a GET, without ensure the id exist in the first place, and so creating it first....

I reckon the complexity and time spent dealing with this would be a bit much.

Couldn't this be an option, maybe a simpler one: collapse multiple blank lines to a single blank line: so that blank lines are just kept/collapsed, instead of adding them after a given tag.

[kieran-ryan]

Thank you for the example of this use case @Jiehong. Understand now why you are requesting this feature.

For me, it feels like one of the tough design decisions for any language or tool where it could be a challenge to support. Cucumber and Karate have taken different and intentioned design directions with their tools. This appears to be a useful feature request from a Karate perspective but arguably it may be difficult to achieve agreement to integrate it with Cucumber.

I found this to be a really interesting issue to spend some time diving into and thinking about some of the possible challenges, which I have captured below. I hope you will enjoy reading it and would be interested in your thoughts on them @Jiehong. Thanks again for raising - hope to see more from you!

Challenges

Agreement on point of preference

Cucumber currently applies the simplest approach: no blank lines between steps. This makes it easier to justify this stylistic approach e.g. "We format test steps without blank lines in between as it is the simplest implementation".

However, if we introduce a new line between every case; our argument shifts to a point of preference e.g. "We format test steps with blank lines in between cases as we believe it will improve readability".

With the second option, there is likely to be less agreement with the justification, as some users may prefer the current style and others may prefer the proposed style. This may be a challenge to achieve consensus on a decision.

Impact to existing users

With this feature implemented, users that run the formatter against their tests would observe changes after updating. As with any change to a formatter, this is a point of consideration.

Aligning with Cucumber design philosophy

In relation to the scenario steps: based on the way I write tests, I would consider each 'case' as an independent behavior (or scenario). I found a helpful StackOverflow answer that explains much better than I could on testing one behaviour at a time and aiming to have one When step. Another answer considers this to be the Cucumber 'recommendation', but I could not locate that reference. If this is indeed the recommendation, then it may align to focus on support for independent behaviours from a formatting perspective - as unfortunate as that is in supporting users that write tests following a different approach.

To demonstrate from the point of view of a user, if I was to send HTTP requests to these APIs, and if POST and GET succeed but DELETE fails; the Scenario would be marked as failed - even though I would consider that two independent behaviours have succeeded (POST and GET).

Where I have previously written or observed scenarios with multiple 'case's, the opinion I have formed is that this often occurs when there is issues in how to handle resources, shared context, or preconditions - each of which I personally find difficult.

Thus encouraging a scenario for each behaviour may align closer with the Cucumber design and to write scenarios that are more deterministic and granular.

Mitigations

I would be really curious to see how the proposed mitigations from @mpkorstanje would work out.

@Jiehong would you consider to give them a try? I would be really interested to see whether they work for you and to consider whether you found any benefits or disadvantages with those approaches?

1. Introduce a second empty comment line to increase whitespace between each case
2. Split scenario into multiple scenarios and use after hooks for cleanup. @mpkorstanje, would you have any useful links that demonstrate this? I'd also like to improve my tests in this area!

[Jiehong]

@kieran-ryan thank you for this detailed answer. It now makes more sense why it's difficult to resolve cucumber and karate use cases here.

1. yes, I guess that's a simple workaround
2. already using afterFeature when needed

In the end, perhaps karate and cucumber would need different formatters to better deal with the differences in approach.

I'm ok to close this issue as-is then.

[mpkorstanje]

@kieran-ryan a brief explanation in pseudo code

When I create a resource
   # Post to create a new resource
   # Check the response was 200
   # Extract the resource id
   # Create an action to delete the resource
   # Push the action into a list
Then the resource can be retrieved by its id
  # Get the resource by its id 
# After
  # For each action in actions
  #  Execute the action

The important part is that for each resource created, a delete action is registered after it's creation. This way all the steps that created something are responsible for getting it cleaned up.

Given an existing resource
   # Post to create a new resource
   # Check the response was 200
   # Extract the resource id
When the resource is deleted
  # Delete the resource
  # Check the response was 201
Then the resource can not be requested 
  # Get the resource
  # Check the code was 404

The scenario for deleting something is pretty similar in execution as for the creation and fetch. But in description it is different.