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

Json formater reports results incorrectly when there is an exception in the Around block #909

Closed
gchinis opened this issue Aug 31, 2015 · 7 comments
Labels
🐛 bug Defect / Bug good first issue Good for newcomers

Comments

@gchinis
Copy link

gchinis commented Aug 31, 2015

Hi,

I want to fail tests that take too long, that's why I am using a timeout in the Around block, as per the documentation. The problem with the json formatter is that when a timeout happens the test step is not marked as failure.

Feature File:

Feature: Really long test

Scenario: Long Test
  Given foo
  Then bar

Step Definitions

Given /foo/ do
  puts "foo"
  sleep 5
end

Then /bar/ do
  puts 'bar'
end
Around do |scenario, block|
  begin
    Timeout.timeout(2) { block.call }
  rescue Timeout::Error => e 
    scenario.fail(e)
  end
end

Output:

[
  {
    "uri": "features/docs/test.feature",
    "id": "really-long-test",
    "keyword": "Feature",
    "name": "Really long test",
    "description": "",
    "line": 1,
    "elements": [
      {
        "id": "really-long-test;long-test",
        "keyword": "Scenario",
        "name": "Long Test",
        "description": "",
        "line": 3,
        "type": "scenario",
        "steps": [
          {
            "keyword": "Given ",
            "name": "foo",
            "line": 4,
            "output": [
              "foo"
            ]
          }
        ]
      }
    ]
  }
]

Expected Output:

[
  {
    "uri": "features/docs/test.feature",
    "id": "really-long-test",
    "keyword": "Feature",
    "name": "Really long test",
    "description": "",
    "line": 1,
    "elements": [
      {
        "id": "really-long-test;long-test",
        "keyword": "Scenario",
        "name": "Long Test",
        "description": "",
        "line": 3,
        "type": "scenario",
        "steps": [
          {
            "keyword": "Given ",
            "name": "foo",
            "line": 4,
            "output": [
              "foo"
            ],
            "result": {
              "status": "failed",
             "error_message": "Timeout...."
            }
          }
        ]
      }
    ]
  }
]

I believe the error occurs because the Formatter::Json#after_test_step is not called, when there is an exception in the around block

This error impacts those who Jenkins because the jenkins plug-in depends on the json formatter working properly.

Versions:
Ruby 2.2.1p85
Cucumber 2.0.2

@mattwynne
Copy link
Member

Thanks for the excellent bug report.

Around hooks are implemented in a peculiar way, so I can imagine how something like this could happen.

@mattwynne mattwynne added 🐛 bug Defect / Bug good first issue Good for newcomers labels Sep 4, 2015
@brasmusson
Copy link
Contributor

The big underlying problem here is in IMHO that with (the current implementation of) around hooks and scenario.fail, a test case can fail without a test step have failed (and been reported to the formatters). The important invariant should be that the result of the test case is the same as the result of the test step that "dominates" the test case result. That is, the result object for the failing test case should be equivalent (except for the duration), with the result object of the one test step that failed.

I have realized that around hooks are not reported as test steps to the formatters, and to some extent deciding when to send before_test_step and after_test_step for an around hook is not straight forward. But as long a an around hook can cause a test case to fail, that need to be done - maybe send both before_test_step and after_test_step after the around hook has executed (with the duration zero in the result).

@mattwynne
Copy link
Member

There's a fundamental problem with the current modelling of Around hooks, for me. Around hooks are not test steps themselves - the entire test case is nested within the Around hook. It's almost like they're test cases that contain test cases, Russian-doll style.

In that (Russian-doll) model, we'd need for the formatters to be able to deal with test cases to fail without any test steps. I can envisage other reasons why test cases without steps might fail (e.g. they fail a new strict rule about scenario names being mandatory, or we have a new rule that says scenarios without steps should be marked as pending). So maybe we should reconsider the invariant and make the formatters able to cope with this situation? WDYT?

@brasmusson
Copy link
Contributor

A conflict here is that the json report does not include the result for a scenario, only the results for all the steps and hooks of it. And introducing the result for the scenario, would probably mean that all existing tools that process the json report would ignore them anyway.

Pragmatically we could change the JsonFormatter to detect if the after_test_case reports the result of the test case as failed, while all test steps were passed, and report that case as a failed after hook. Then existing tools should be able to detect the failure, even tough it is reported in a not entirely correct way.

@mattwynne
Copy link
Member

I see. That sounds like a good workaround.

We could even do that deeper within Cucumber (effectively modelling the Around hook code as a TestStep), but I wonder whether that would just end up as confusing...?

@brasmusson
Copy link
Contributor

One questing from this example is when using an around hook to wrap the execution of a test case in a timeout, will the formatters still receive paired before_/after_ calls, or will the experience this (which is indicated from that there are no result for the Given step in the json file):

before_test_case
  before_test_step
    <the timeout triggers>
after_test_case

In that case the invariant that if before_ has fired for a test case or test step, then also after_ will fired for that test case or test step, is violated.

brasmusson added a commit to cucumber/cucumber-ruby-core that referenced this issue Sep 6, 2015
A timeout in an around hook can interrupt a running test step. From the
perspective of formatters, the invariant that both before_test_step and
after_test_step is received for each step, should hold also in this
case. Fixes the main part of cucumber/cucumber-ruby#909.
@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
🐛 bug Defect / Bug good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants