gherkin: Compiling empty scenarios

[mattwynne]

Summary

Currently, the Gherkin pickles compiler removes Gherkin scenarios that don't have any steps, so they don't appear as test cases.

I understand why this was done - what's the point in running a test case that isn't going to test anything? - yet I think we're missing a thing here.

When I watch teams do BDD, they very often start out by creating just the headline or title of a scenario, and defer filling out the Given/When/Then steps until later. I'd like it if Cucumber were reporting that scenario's status as undefined. In order to do this, we'd need to pass a pickle out of the compiler.

Expected Behavior

Pickles compiler outputs pickles (with no steps) for scenarios with no steps.

Current Behavior

Pickles compiler does not output a pickle for scenarios with no steps.

Possible Solution

I'd love some pointers here. I've never worked on the pickles compiler, but if someone can show me where to start (and we're agreed on the idea), I'd have a go at this.

Context & Motivation

I'm thinking about how people like product owners engage with Cucumber, and that I'd guess they think of a scenario as having taken life as soon as they name it. It seems wrong to hide those scenarios from Cucumber.

[aslakhellesoy]

Consider there two feature files:

Feature:
  Background:
    Given foo

  Scenario: empty

Feature:
  Scenario: not empty
    Given foo

The first one should be undefined and the second should not. But if we send all pickles through, how would we distinguish between the two?

[aslakhellesoy]

Hi @mattwynne

The compiler is pretty simple, compiler.rb is less than 200 LOC.

It doesn't have unit tests, though - only approval tests (the .pickles.ndjson files).

I would start with these files:

• Input: https://github.com/cucumber/cucumber/blob/master/gherkin/testdata/good/incomplete_scenario.feature
• Expected output: https://github.com/cucumber/cucumber/blob/master/gherkin/testdata/good/incomplete_scenario.feature (currently empty - that't what we want to change)

Rather than changing the expected output by hand, I'd make the change you think is correct. Start with the Ruby implementation - you can do the other languages later. The fix is maybe to remove these lines:

• https://github.com/cucumber/cucumber/blob/88ee9254838bd0ff77249665e7eccf1a1c588687/gherkin/ruby/lib/gherkin/pickles/compiler.rb#L27
• https://github.com/cucumber/cucumber/blob/88ee9254838bd0ff77249665e7eccf1a1c588687/gherkin/ruby/lib/gherkin/pickles/compiler.rb#L48

Now, cd into cucumber/gherkin/ruby and run make. You should get an error because the generated pickles are different from the expected (empty) one.

Make deletes the generated file if the test fails (its presence indicates "test passed" and prevents make from running it next time). Disable that temporarily by commenting out .DELETE_ON_ERROR in the Makefile so that we can inspect the file after a failure.

Run make again and inspect the generated file, then remove it so the test will run next time.

jq acceptance/testdata/good/incomplete_scenario.feature.pickles.ndjson
rm acceptance/testdata/good/incomplete_scenario.feature.pickles.ndjson

Does it look correct? Do you need to make more changes to the code?

When you're happy, save it to the golden master:

mv acceptance/testdata/good/incomplete_scenario.feature.pickles.ndjson ../testdata/good/

After copying the new golden master over here, sync it to all the other gherkin implementations:

cd ../..
source scripts/functions.sh
rsync_files

When you run git status you'll see that the updated golden master has been updated in all the other language implementations too.

This will of course cause the other Gherkin implementations' tests to fail, so now it's time to go and fix those. It's easier than you think - the structure is the same so you can hack on languages you don't know!

I recommend fixing all implementations in the same branch/PR, one commit at a time. If you're stuck in a language, someone else will jump in and do it for you. Do try though.

[brasmusson]

The first one should be undefined and the second should not. But if we send all pickles through, how would we distinguish between the two?

By producing an empty Pickle when the Scenario or Scenario Outline AST object has not steps (regardless of the existence of a Background with steps)?

[aslakhellesoy]

Ah yes of course!

[mattwynne]

The alternative would be for the compiler to just diligently compile the test case as usual, and leave it up to the higher-level bits (e.g. the runner) to figure out that this step is from a background and still give the scenario an undefined status.

Seems a little bit icky to me to have the compiler deciding about things like that.

I'm happy to let this edge-case come out in the wash, TBH.

[aslakhellesoy]

I agree with @brasmusson - if there are no steps in the scenario, no steps in the pickle regardless of how many steps there are in the background.

I don't see why you want to leave that decision to individual cucumber implementations when it can be done consistently in the Gherkin lib @mattwynne.

Also seems like it would be hard for cucumber to determine whether pickle steps came from background or not.

[brasmusson]

Currently the compilers treat the edge-case for a scenario with no steps (by not creating a Pickle) the same way regardless of any background content. So changing the outcome of that edge-case to creating an empty Pickle (regardless of any background content), should fit in nicely in the current compiler structure.

[aslakhellesoy]

I might not get to that until next week, so have a go at it if you want @brasmusson!

[enkessler]

I'm late to the party on this one but, for what it's worth, I see outlines test generators and backgrounds as test adjusters. So not pickling an outline without example rows makes sense because there are no concrete tests to wrap up. Similarly, a feature with only a background shouldn't pickle anything because it would be completely arbitrary to say what number of pickles should be made for anything besides 0 because there are still no actual tests to attach to. Once a scenario is added (whether it be direct with Scenario: or indirect with an outline plus example row), however, it can be said that there is a test and thus something to pickle up.

If I understand the point of pickles, they are the 'resolved' form of a test. We don't produce one pickle for an outline, we 'resolve' it and produce one pickle for each example row. We don't pickle up a scenario and then have an extra object for the background steps, we 'resolve' it and produce one pickle with all steps included.

This being the case, I would expect one pickle per Scenario: or non-parameter row in an Examples:. and for those pickles to include all applicable steps, regardless of where they came from.

[aslakhellesoy]

That!s a nice explanation @enkessler and I believe this is how it currently behaves.

[enkessler]

@aslakhellesoy According to our test data, no.

Previous behavior: no pickles created for a scenario without steps. Current behavior: pickle created for a scenario without steps but steps from a background are left out of the pickle Expected behavior: pickle created for a scenario without steps and steps from a background are included in the pickle.

[aslakhellesoy]

I disagree.

Imagine you have this:

Feature: Go shopping

  Background: we have some money
    Given I have £10

  Scenario: spend some of it
    When I buy a paper for £1
    Then I should have £9 left

  Scenario: spend some of it
    When I buy a jumper for £11
    Then I should be denied
    But I should have £10 left

Now we want to add some more scenarios, without fleshing them out, because we need to do some more analysis. We're adding them more as a note to remind ourselves that we have some more work to do.

Feature: Go shopping

  Background: we have some money
    Given I have £10

  Scenario: spend some of it
    When I buy a paper for £1
    Then I should have £9 left

  Scenario: can't afford what we want
    When I buy a jumper for £11
    Then I should be denied
    But I should have £10 left

  # New scenarios

  Scenario: get a discount

  Scenario: got a loyalty card with a free gift

We don't want those new scenarios to run at all, not even with the background. It potentially slows down the build, and it's pointless - there are no steps in the scenarios themselves. There is nothing new to learn from executing these scenarios.

We do however want the report to contain information that we had 4 scenarios, 2 of which are undefined. We can represent that with empty pickles.

If we did what you're suggesting, Cucumber would have to execute the 2 empty scenarios. It can't tell that its sole step came from a background and none from the scenario and decide not to execute it.

[enkessler]

Cucumber executing a test that got written down is a feature, not a bug. If you don't want a test to to run because it isn't finished yet, that is what filters are for. Slap a @wip tag on it and call it a day.

# New scenarios

@wip
Scenario: get a discount

@wip
Scenario: got a loyalty card with a free gift

If nothing else, having different behavior for this edge case is inconsistent. As soon as you add a single step in the Scenario: it gets treated normally and is going to start getting executed whether it is 'finished' or not, so the burden of having their test suite be able to accommodate for unfinished tests remains on the test writers. This is the right place for that burden because when a test is complete can only be known by the author. Maybe it is done after one step, maybe it will be done in another ten steps. Maybe it's another case of really bad use of the tool and they really do mean the test to have no additional steps (we've all seen sillier things over on the forums).

Admittedly, the lack of any steps in a test aside from background ones is highly suspicious but if you want a tool warn you about suspicious things, that's what linters are for*. A compiler's job is to take in your source code (Gherkin) and spit out something actionable (Pickles), silly though that action may be.

TLDR: preventing those test from running is someone else's job

*We should really get back around to writing that thing.

[enkessler]

@aslakhellesoy Fun fact: I ran into this use case just the other day.

I had a couple complete scenarios that had common enough setups that a Background had been made. I also had another half dozen Scenarios that I had yet to finish (marked with a @wip, naturally). Being able to resume working on the unfinished tests by just removing the @wip tag and running them in order to see what to do next was nice. A nicer alternative than having to add a fake step to the test in order to get it to run only to then have to replace with a real step a second later.

If I upgrade to Gherkin 5 I will no longer have this nicety. :(

[lock[bot]]

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.

[mpkorstanje]

So I ran into this with Cucumber JVM.

The general rule is that the state of a scenario is the worst state of its executed steps and passing (the best state) if no steps have been executed yet. By making this undefined two rather annoying edge cases are introduced:

1. The first time a before hook is invoked the state of the scenario was undefined.
2. When a scenario is undefined, there should be undefined steps and snippets to print because each undefined step generates a snippet. Unless it is the empty scenario.

Because we're deprecating non-strict mode undefined steps will always result in a failure. So I would suggest making empty scenarios fail. Perhaps with a pending exception/state.

[aurelien-reeves]

At the moment, it looks like the gherkin parser is parsing empty scenarios as requested in the initial message of that issue.

cucumber-ruby is reporting the scenario as undefined. cucumber-jvm is reporting the scenario as passed. cucumber-js is reporting something like "2 scenarios (1 passed)" when no hooks are set. It is reporting it passed when hooks are executed.

So, what needs to be done now? Should we try to aligning cucumber-jvm and cucumber-js on cucumber-ruby? Should that be part of that same issue? Maybe we could close it and open new one in cucumber-jvm and cucumber-js?

[aurelien-reeves]

As discussed with Aslak, we'll open a PR with an empty scenario in the compatibility kit The expected result should be the scenario to be undefined, and no hooks to be executed - like cucumber-ruby

[mattwynne]

👍 let's make sure to link from this issue to the PR once it's open so people can follow the trail.

[mattwynne]

Personally I would keep this issue open until the PR is merged but I don't feel strongly about that.

[aurelien-reeves]

https://github.com/cucumber/cucumber/pull/1498

[aslakhellesoy]

Reopening this because of discussion in Slack

@davidjgoss:

What's the consensus across implementations about scenarios that contain no steps? This seems to break cucumber-js, not sure if it's worth addressing https://github.com/cucumber/cucumber-js/issues/1668

@mpkorstanje:

Well, Aslak seems to think it's a good idea to have the empty scenario result in an undefined result. I think that empty scenarios should be treated like an ordinary scenario. Execute all the before/after hooks, but not any steps (because there are none). It makes it much easier to keep everything consistent (esp when dealing with events, scenario outcomes, ect). If empty scenarios are undesirable that should be handled by a linter.

@aslakhellesoy:

I don't have a strong opinion about it, just that we implement it consistently.

[enkessler]

I think that empty scenarios should be treated like an ordinary scenario. Execute all the before/after hooks, but not any steps (because there are none). It makes it much easier to keep everything consistent (esp when dealing with events, scenario outcomes, ect). If empty scenarios are undesirable that should be handled by a linter.

I like all of those words. My only question is whether or not steps from a Background count or not.

[aurelien-reeves]

We may let users choose how they would process their empty scenarios with an option:

• execute all hooks and background steps, and report the scenario with a status depending the execution of those background steps and hooks
• do not execute anything and report it as undefined skipped|empty|pending|...

I think both options may find their audience.

Also, I was curious to know how other tools consider empty tests. I have checked the behavior of rspec. It shows a "Not yet implemented" and report the test as pending. Hooks are not executed: if an error is raised, implemented tests are reported as "Error", but the not implemented test is still reported as pending.

$ bundle exec rspec spec/cucumber/cli/main_spec.rb:23 -fd
Run options: include {:locations=>{"./spec/cucumber/cli/main_spec.rb"=>[23]}}

Cucumber::Cli::Main
  #execute!
    passed an existing runtime
      configures that runtime (FAILED - 1)
      uses that runtime for running and reporting results (FAILED - 2)
      do something (PENDING: Not yet implemented)

Pending: (Failures listed here are expected and do not affect your suite's status)

  1) Cucumber::Cli::Main#execute! passed an existing runtime do something
     # Not yet implemented
     # ./spec/cucumber/cli/main_spec.rb:50

Failures:

  1) Cucumber::Cli::Main#execute! passed an existing runtime configures that runtime
     Failure/Error: raise "error"

     RuntimeError:
       error
     # ./spec/cucumber/cli/main_spec.rb:10:in `block (2 levels) in <module:Cli>'

  2) Cucumber::Cli::Main#execute! passed an existing runtime uses that runtime for running and reporting results
     Failure/Error: raise "error"

     RuntimeError:
       error
     # ./spec/cucumber/cli/main_spec.rb:10:in `block (2 levels) in <module:Cli>'

Finished in 0.00136 seconds (files took 0.75502 seconds to load)
3 examples, 2 failures, 1 pending

Failed examples:

rspec ./spec/cucumber/cli/main_spec.rb:30 # Cucumber::Cli::Main#execute! passed an existing runtime configures that runtime
rspec ./spec/cucumber/cli/main_spec.rb:40 # Cucumber::Cli::Main#execute! passed an existing runtime uses that runtime for running and reporting results

Do you know other tools which may be compared to cucumber as a BDD test runner? I think we may comply to what is done with our "competitors" if some kind of consensus has already been found elsewhere.

[aslakhellesoy]

While we could let users choose the behaviour, I don't think we should. It introduces complexity without significant benefit.

I think we should choose one behaviour and implement it consistently.

[mpkorstanje]

For what it's worth, considering the empty scenario as a scenario with 0 steps that will execute before/after all and before/after scenario hooks and is passed by default is the most consistent and least complicated solution.

1. Hooks have access to the current scenario state. This can be elegantly defined as the worst result of all previous steps/hooks or passed if none.
2. So for consistency, after completion the scenario result can be defined as the worst result of all steps/hooks or passed if there was no result.
3. Undefined steps result in a suggested step definition snippet event. So for every undefined scenario there is at least one suggested snippet. By defining the scenario result as passed when there are no step (rather then undefined) we ensures that for an undefined step/scenario there is always at least one suggested snippet. This removes an edge case for implementers of plugins.

[aurelien-reeves]

The fact that a scenario is empty is an important information. It may even be an very important information in a BDD workflow, or within a living documentation. Reporting it as passed or failed is loosing that piece of information.

From a testing tool point of view, I know some tools which reports empty tests or tests without expectation, and requires the developer to explicitly specify that no assertion is actually expected. As annoying as it can be for the developer, it make it easier to retrieve that info. And for code maintenance, it also make it explicit that the test is empty or has no expectation on purpose.

[davidjgoss]

I'm thinking about the principle of least surprise for this one. As a user, given my intent is something like "putting this scenario title here as a placeholder, coming back later to flesh it out" (see early comments), I think I'd be more surprised if Cucumber ran the hooks and background steps.

[mpkorstanje]

Following the principle of least surprise everything that happens before a scenario should happen before a scenario with no steps. The latter being a subset of the former.

As an analogue: Scenarios are the the equivalent of tests. Features the equivalent of a class with multiple tests. A test with no statements in the method body is equivalent to an empty scenario.

Feature: Test
  Scenario: nothing

Would be equivalent to:

class Test {

  @BeforeEach
  public void before(){

  }

  @AfterEach
  public void before(){

  }

  @Test
  public void nothing(){

  }
}

This is a valid junit test and will execute all hooks. If you have a decent linter, it will complain about a test without an assertion.

a BDD workflow,

We can't and shouldn't try to facilitate all possible work flows during test execution. That's what linters are for. It also solves the problem of making things optional or not. Linters can usually be configured with rules about things they complain and don't complain about.

[davidjgoss]

I don't really see a Scenario as being analogous to a JUnit test. My angle was that it only becomes a runnable test once I actually add some steps and implement the glue for them - before that its just a piece of unfinished documentation. This is maybe not the most prevalent point of view though...

[mpkorstanje]

My angle was that it only becomes a runnable test once I actually add some steps and implement the glue for them

So you are arguing that an empty scenario should not result in a pickle and completely ignored when it comes to execution and listing scenarios?

[davidjgoss]

For me, yes - but this does contradict @mattwynne original point about not having visibility of these empty scenarios if they never manifest as pickles.

(Conscious that I'm not really adding anything new here, so going to ease up a bit :))

[aurelien-reeves]

As an analogue: Scenarios are the the equivalent of tests. Features the equivalent of a class with multiple tests. A test with no statements in the method body is equivalent to an empty scenario.

Feature: Test
  Scenario: nothing

Would be equivalent to:

class Test {

  @BeforeEach
  public void before(){

  }

  @AfterEach
  public void before(){

  }

  @Test
  public void nothing(){

  }
}

This is a valid junit test and will execute all hooks. If you have a decent linter, it will complain about a test without an assertion.

Yes, this is a valid and passing junit test.

But the equivalent is not a valid and passing rspec. Mocha would also report it as a pending test. PhpUnit also has a specific behavior for test without any assertion in it.

So we should not rely only on how junit behaves to decide how cucumber should behave I guess.

[mpkorstanje]

I don't think your fiddle worked. It's not showing me a pending test.

[aurelien-reeves]

I don't think your fiddle worked. It's not showing me a pending test.

Fixed :D

[mpkorstanje]

PhpUnit also has a specific behavior for test without any assertion in it.

PHP Unit does this by checking after the test was executed if any assertions were invoked. This is essentially a build in after hook. PHP Unit can do this because they provide their own assertion library. In Java there are many to chose from (most linters just check for the word assert).

Fixed :D

If you pass in an empty function, it does pass. :laughing:

mocha.setup("bdd");
chai.should();

describe("Test", function() {
    it("nothing", function(){

    });
});

mocha.run();

And mocha will execute hooks, even if the test is pending:

mocha.setup("bdd");
chai.should();

describe("Test", function() {

     before(function() {
       throw "oops"
    });

    it("nothing");
});

mocha.run();

Though marking the test as pending rather then undefined solves the problems with objection cucumber/common#3. It does leave cucumber/common#1 and cucumber/common#2 in place.

[mattwynne]

@mpkorstanje there's something you said that I don't understand:

Undefined steps result in a suggested step definition snippet event. So for every undefined scenario there is at least one suggested snippet.

I don't see how we'd have an undefined step here since there are no steps. What did I miss?

Let's use a concrete example, we like those 😁

login.feature

Feature: Login

  Background:
    Given a user Dave

  Scenario: Successful login
    When Dave logs in with his password
    Then he can access the dashboard

  Scenario: Failed login

The Failed login scenario is blank because the team are still deciding how to handle a failed login attempt, so they leave it blank.

Let's imagine there's also a hook in this suite to reset the state of the database between tests.

hooks.js

const { Before } = require('@cucumber/cucumber')

Before(() => Database.reset)

What I'd like to have happen is that the team can see in their results that there are two scenarios, and one of them is "not finished". undefined seems ideal to me, but pending would also work:

2 scenarios (1 passed, 1 undefined)

I can see that considering the case of the background steps adds complexity - I don't know if we have information in the pickle steps about whether a step came from a background or not - so it might be nice to ignore that one for now. I do think we should deal with hooks though. I don't think this would be very useful otherwise.

[mattwynne]

I can imagine how we'd implement this in cucumber-ruby, once the pickle compiler is passing these empty scenarios through. Perhaps @aurelien-reeves and I should pair on this and we can show you what we come up with, for discussion.

[mpkorstanje]

The proposal is the following rule:

• The scenario state is "undefined" if no steps have been executed.

This comes on top of the existing rule:

• The scenario state is the worst state of the executed steps.

I'm making a two pronged argument as to why this is a bad idea.

Prong 1:

For example you write a plugin that prints a summary of a scenario executions. To make this possible, whenever an undefined step is encountered a suggested snippet event is emitted. So there is a third rule:

• For undefined step there is an suggested snippet event.

So you could collect all these suggested snippets for each scenario, and when a scenario finishes with the status "undefined" you print the list of suggested snippets. BUT if there are no snippets, the scenario had no steps and a different message must be printed. And that's a really weird edge case to deal with when writing a plugin.

The reason you have this edge case is because there are two ways for a scenario to get the "undefined" state:

1. Either the execution of any of the steps end with the "undefined" state.
2. Or the scenario did not have any steps and defaulted to "undefined".

You could work around this by instead proposing:

• The scenario state is "pending" if there were no steps.

But this will run into problems the second prong.

Prong 2:

Before- and after-hooks have access to the current scenario state. This for example allows them to take screenshots when a scenario did not pass. Following the the proposed rule the first before hook will see the scenario state as "undefined" because no steps have yet been executed.

You could work around this by defining the default scenario state from the perspective of the consumer:

• The scenario state provided to a hook is "passed" if no steps have been executed.
• The scenario state provided to an event is "undefined" if no steps have been executed.

Implementation wise this is also kinda annoying because now depending on the consumer you have to calculate the state of the scenario differently. But more importantly this results in an inconsistency between hooks and event consumers when dealing with the empty scenario. The last after hook would see the scenario as passed while the event consumer would see the scenario as undefined.

And of course there are ways to work around this and add more and more nuance. However by doing this we keep adding more and more complexity. Complexity that we absolutely shouldn't have to deal with to finesse what amounts to an edge case. In nearly every other test framework in existence the empty test passes by definition.

Hence my proposal:

• The scenario state is "passed" if no steps have been executed.
• The scenario state is the worst state of the executed steps.

This allows us to treat the empty scenario as any other scenario, but with an empty list of steps. Before- and after-hooks are executed as the would for any other scenario with steps. Hooks and and event consumers can be build on the same assumptions. There is no weird interaction with suggested snippet events and the whole thing can be expressed elegantly as:

private final List<Result> stepResults = new ArrayList<>();

public Status getStatus() {
    if (stepResults.isEmpty()) {
        return Status.
    }
    return max(stepResults, comparing(Result::getStatus)).getStatus();
}

[mpkorstanje]

Let's use a concrete example, we like those. I can see that considering the case of the background steps adds complexity

So far I've been talking about compiled pickles with no steps. You've gone down a different rabit hole with your example:

For everything I wrote the following example can be used:

Feature: Login

  Scenario: Successful login
    Given a user Dave
    When Dave logs in with his password
    Then he can access the dashboard

  Scenario: Failed login

Here the failed login scenario has an empty list of steps.

However for this example:

Feature: Login

  Background:
    Given a user Dave

  Scenario: Successful login
    When Dave logs in with his password
    Then he can access the dashboard

  Scenario: Failed login

The question should be: "Does the Failed login scenario compile into a pickle with 0 or 1 steps".

I'm somewhat ambivalent about either. With what I've been proposing so far regardless if the scenario has 1 or 0 steps, the outcome would probably be passed. However with the other proposal the outcome might also be undefined/pending. And then this distinction matters because it would flag a potential problem in the workflow.

[mpkorstanje]

Anyway, I get the impression that all arguments for pending/undefined stem from the need to facilitate a test driven workflows where scenarios are intentionally incomplete.

However we are discussing Cucumbers execution model and implicitly its event model. We are also trying to keep it consistent across multiple implementations and integrate it with various test runners and IDEs. What ever complexity we add here is expensive, time consuming and very hard to change. This means we need simple and well defined behaviors.

Yet workflows are fuzzy and mostly involve people. This means we need configuration, flexibility and nuance. The execution model is the wrong abstraction layer to address these concerns. By the time we get this deep into Cucumbers guts workflow issues should have been dealt with.

This is why I've been proposing a linter. Linters are the ideal tool to handle workflows. They don't require Cucumber to be executed. This makes them light weight in terms of dependencies and allows them to run while writing features. This removes a lot of complexity from the linter and potentially makes them language agnostic.

Linters tend to come with configurable rules and a set of of defaults. This makes them ideal for dealing with individual workflows. Linters usually also tend to have machine consumable output for easy integration with IDE's and other tools. All things we can't really cram into Cucumbers execution model and shouldn't try.

[mpkorstanje]

What did I miss?

I'd say about 2-3 years of working on Cucumber. :stuck_out_tongue:

Like Cucumber Ruby, Cucumber JVM used to default to "Undefined" for empty scenarios. Along with strict/lenient-mode it has been an endless source of bugs and confusion. So I removed both from Cucumber JVM. I did this by defaulting to strict mode, removing lenient mode and treating empty scenarios as passed by definition. Now empty scenarios never fail the build while undefined scenarios always fail the build.

In practice it hasn't made a lick of a difference for the end user. In the JVM ecosystem tests frameworks only support three test outcomes; passed, skipped, or failed. And in CI this is reduced to either passed or failed. So Cucumbers very nuanced test outcome with six possible states is ultimately reduced to a binary or trinary outcome.

Simply not bothering with some of the nuance removed a bucket of complexity. This has yet to be a source of complaints. The bugs and inconsistencies on the other hand were. So I think it is important to look at this holistically.

[aurelien-reeves]

In practice it hasn't made a lick of a difference for the end user. In the JVM ecosystem tests frameworks only support three test outcomes; passed, skipped, or failed. And in CI this is reduced to either passed or failed. So Cucumbers very nuanced test outcome with six possible states is ultimately reduced to a binary or trinary outcome.

Simply not bothering with some of the nuance removed a bucket of complexity. This has yet to be a source of complaints. The bugs and inconsistencies on the other hand were. So I think it is important to look at this holistically.

Good point here: maybe we should not try harder to report empty scenarios consistently. As you said, it may depend on the ecosystem.

As it would make perfect sense to have empty scenarios reported as "pending" in ruby and JS, if it make absolutely no sense in Java (even the "skipped" one?), maybe we should stop trying do have the exact same behavior for it.

That should not prevent the common packages to be consistent as the definitive status of a test remain hardly tied to the implementation itself

[enkessler]

Speaking of concrete examples, this is a reminder that, whatever is eventually decided, the test data will need to be updated (and, unless I am misinterpreting that expected output, the test has been inaccurate for years).

https://github.com/cucumber/common/blob/master/gherkin/testdata/good/incomplete_scenario.feature https://github.com/cucumber/common/blob/master/gherkin/testdata/good/incomplete_scenario.feature.pickles.ndjson

Edit: To clarify, it looks to me like a pickle is getting created despite the feature file verbiage indicating that no pickle should get created.

[mattwynne]

You've made a lot of points Rien!

I appreciate the history you've been through with Cucumber-JVM since I raised this ticket back in 2017, and the clarity of thinking you can bring to this problem.

I see two sides to this problem:

1) When we compile an "empty" Gherkin scenario into a pickle 2) If/when we try to execute a pickle for an empty scenario

The original intent of this ticket was to focus on (1) - that's why I raised it in this repository, where the compiler is - because at the time empty scenarios were effectively "censored" from Cucumber by the compiler. Any debate about how to handle their execution and results would be moot because they didn't even appear in the message stream.

My feeling is that although a linter is probably a good idea to have at some point, linting is a long way away at the moment, and it still makes sense for us to think about what is the right way for the compiler to express these edge cases.

I'd like to clarify what the current behaviour of the compiler is here, as it may have changed since I raised the ticket. As @enkessler has pointed out, it also seems to be doing some stuff that's inconsistent with what we'd intended.

[mattwynne]

For compilation, my take on the rules would be (and I don't know what the current behaviour is yet):

Gherkin	Pickles / Messages
Empty scenario	{ "pickle": { ... "steps": [] ... } }
Empty scenario in a feature with a background	{ "pickle": { ... "steps": [] ... } }[1]

[1] i.e. the background steps are only slurped up into scenarios that are non-empty)

At a common-sense/POLS level, those rules seem to make sense to me.

The one I'm not sure about is a scenario outline. If we have a scenario outline with steps but no examples, should that also be represented as a single empty pickle?

[enkessler]

The one I'm not sure about is a scenario outline. If we have a scenario outline with steps but no examples, should that also be represented as a single empty pickle?

I would expect an outline with no examples to generate no pickles because an outline is merely a template from which actual scenarios are generated. An empty scenario, on the other hand, is at least a definite thing. We could debate on whether it should be a pickle with no steps or a pickle with some steps (inherited from a background) but we can say that it at least exists as a discrete test case on which a consumer might decide to take some action.

Lacking example rows, however, an outline cannot be said to be any definite thing. We could justify making a thousand pickles from it as easily as justifying just one pickle because it's all being made up from nothing anyway. Zero pickles is a reasonably safe interpretation of no examples rows.

[mpkorstanje]

For compilation, my take on the rules would be (and I don't know what the current behaviour is yet):

Might be good to do your homework first. :stuck_out_tongue:

[enkessler]

For compilation, my take on the rules would be (and I don't know what the current behaviour is yet):

Might be good to do your homework first. 😛

If we take the idea of "Write the code you wish you had" up a level and make it "Write the requirements you wish you had", then not knowing the current behavior might actually be a good thing. It could free one from the temptation of altering the requirements to reduce the effort needed to implement a change rather than sticking to the desired goal and worrying about how to get there until after it is settled.

[aurelien-reeves]

The PR cucumber/common#1498 is ready for review.

It does not change any behavior in runtime code. It just add an acceptance tests of what is already happening.

Fake cucumber has been updated a little bit to match the behavior of cucumber-ruby because cucumber-ruby already has an explicit support for empty scenarios, cucumber-js has not - and also it was far more easy to do so due to how acceptance test-data are managed in the monorepo at the moment.

Also, I suggest to merge that PR not with the idea to impose the actual behavior neither to enforce the same behavior on all implementation, but to have some explicit spec for what is already happening now within the mono-repo.