-
Notifications
You must be signed in to change notification settings - Fork 60
Split pending and skipped tests #34
base: master
Are you sure you want to change the base?
Conversation
|
|
ping @christian-bromann |
Let's make sure first to have all reporters updated before landing this change. |
@@ -84,7 +84,8 @@ class CucumberReporter { | |||
case Cucumber.Status.PENDING: | |||
case Cucumber.Status.SKIPPED: | |||
case Cucumber.Status.AMBIGUOUS: | |||
e = 'pending' | |||
e = 'skipped' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@just-boris I still don't know why we change the event name here, can't we handle this in the allure reporter only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pending status is still there for really pending tests (tests with empty implementation)
According to this change, pending, skipped and ambiguous tests are still treated equally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point of this change is normalizing behavior of Cucumber with other frameworks.
Neither Mocha nor Jasmine doesn't emit test:start
for pending tests.
I tried to cancel test:start
event for pending test, but Cucumber API is designed in a way that doesn't let us know about pending test in advance.
This is why I came up with the current solution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that the base reporter does not trigger if the event name is skipped
, see https://github.com/webdriverio/webdriverio/blob/master/lib/utils/BaseReporter.js#L114
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the allure reporter is the only reporter that has problems with it why not just fixing it there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a valid point, but then here is a different problem.
Mocha framework doesn't fire test:start
event for pending tests, but Cucumber does.
Allure is the only reporter that suffers from it just because it is the only reporter from the standard set, that uses this event, but there is a general problem in inconsistency, that I am trying to mitigate.
I just bumped into this issue. Allure is not able to generate the reports from our cucumber features and this patch seems to fix the issue. As I understand the malformed XML files are missing How could I help? |
@esclapes @just-boris we need to mitigate the inconsistencies between all frameworks and make sure that they all fire the same events |
I have had a look at the different framework adapters to check how/when are they emitting events. There are indeed some inconsistencies. I might have missed something, but here are my two cents:
This is the overview.
|
This is a first step to solve webdriverio-boneyard/wdio-allure-reporter#26
Now skipped tests will be reported with
skipped
status. Thepending
status is still there for really pending tests (tests with empty implementation).