Skip to content
This repository has been archived by the owner on Dec 18, 2019. It is now read-only.

Fix support for Scenario Outlines #126

Closed

Conversation

goofballLogic
Copy link
Contributor

Scenario outlines require parsing the gherkin document's Examples hash in addition to the Steps hash to determine the location of an executing step

@christian-bromann
Copy link
Contributor

@goofballLogic can you rebase on top of current master? I already merged your previous PR.

@BorisOsipov BorisOsipov self-requested a review May 17, 2018 15:34
@goofballLogic
Copy link
Contributor Author

Related to #111

@christian-bromann
Copy link
Contributor

@goofballLogic can you do:

$ git rebase origin master

There seems to be rebase conflicts and I would like to avoid a merge commit.

@christian-bromann
Copy link
Contributor

nevermind, I can also just squash it

@BorisOsipov
Copy link
Contributor

@goofballLogic it still fails on my example.
https://github.com/BorisOsipov/wdio-cucumber4-issue

TypeError: Cannot read property 'tests' of undefined
    at ReporterStats.testStart (C:\Temp\wdio-cucumber4-issue\node_modules\webdriverio\build\lib\utils\ReporterStats.js:348:74)
    at BaseReporter.<anonymous> (C:\Temp\wdio-cucumber4-issue\node_modules\webdriverio\build\lib\utils\BaseReporter.js:151:25)
    at emitOne (events.js:115:13)
    at BaseReporter.emit (events.js:210:7)
    at BaseReporter.handleEvent (C:\Temp\wdio-cucumber4-issue\node_modules\webdriverio\build\lib\utils\BaseReporter.js:300:27)
    at Launcher.messageHandler (C:\Temp\wdio-cucumber4-issue\node_modules\webdriverio\build\lib\launcher.js:688:28)
    at emitTwo (events.js:125:13)
    at ChildProcess.emit (events.js:213:7)
    at emit (internal/child_process.js:774:12)
    at _combinedTickCallback (internal/process/next_tick.js:141:11)

@BorisOsipov
Copy link
Contributor

I guess the problem that
https://github.com/webdriverio/wdio-cucumber-framework/blob/master/lib/reporter.js#L237

getUniqueIdentifier generates different ids for the same steps\tests

@goofballLogic
Copy link
Contributor Author

@BorisOsipov yes you're right. Some sort of behaviour is required of this module for which there is no test. I'd have to reverse engineer the webdriverio module to understand what the requirement is.

The use of target.line is problematic becuase in a Scenario Outline, each example is effectively its own Scenario (similar with Background). The current code seems to assume that Scenarios descend directly from Features, but with Cucumber there is more than one level of nesting.

I'll see if I can work out what webdriverio requires...

@goofballLogic
Copy link
Contributor Author

Having dived a bit deeper, I'm concluding that there are some design decisions which need to be made here.

For a normal scenario like:

Feature: my feature
 Scenario: my scenario
  Given step 1
  When step 2
  Then step 3

The hierarchy is:

Feature: my feature
  - Scenario: my scenario
       - Step: Given step 1
       - Step: Given step 2
       - Step: Given step 3

As currently designed, for reporting purposes a Step is mapeped to wdio's "test-case", and a Scenario is mapped to wdio's "suite".

However, for a scenario outline like:

Feature: my feature
 Scenario Outline: my scenario outline
  Given step 1 for <thing>
  When step 2
  Then step 3
 Examples: Big things
  | thing      |
  | Elephant   |
  | Whale      |
 Examples: Small things
  | thing      |
  | Mouse      |
  | Bacterium  |

the hierarchy is:

Feature: my feature
   - Scenario Outline: my scenario outline
      - Examples table 1: Big things
         - Example row 1: thing=Elephant
            - Step: Given step 1 for Elephant
            - Step: When step 2
            - Step: Then step 3
         - Example row 2: thing=Whale
            - Step: Given step 1 for Whale
            - Step: When step 2
            - Step: Then step 3
      - Examples table 2: Small things
         - Example row 1: thing=Mouse
            - Step: Given step 1 for Mouse
            - Step: When step 2
            - Step: Then step 3
         - Example row 2: thing=Bacterium
            - Step: Given step 1 for Bacterium
            - Step: When step 2
            - Step: Then step 3

So wdio's "test-case" should still map to a Step, but then we are faced with a problem - we need to fit a three-level heirarchy into wdio's concept of "suite".

The easiest path seems to be that each Example Row be mapped to a wdio "suite", so that reporting can still function. So a suite would be named something like "my scenario outline, examples 1 (Big things), row 1".

I actually have this working in the local copy of my fork, but there are quite a few implicit design decisions I'm steamrolling through. How do the maintainers ( @christian-bromann ?) usually make design decisions like this? Should there be a discussion elsewhere?

@BorisOsipov
Copy link
Contributor

@goofballLogic it will be better if new reporter handles scenario outline as old one (v1).

I can share events log for your feature file example generate from v1 reporter version https://pastebin.com/SbjPK1nh

The easiest path seems to be that each Example Row be mapped to a wdio "suite", so that reporting can still function. So a suite would be named something like "my scenario outline, examples 1 (Big things), row 1".

if I understood your idea is the same as old behaviour.

Hope it helps you with this trickly issue.

@goofballLogic
Copy link
Contributor Author

goofballLogic commented May 19, 2018 via email

@goofballLogic
Copy link
Contributor Author

@BorisOsipov I've had a look through the pastebin, and consolidated the json events into a table showing the distinctive bits here: https://jsfiddle.net/ayxsz00d/

I'm not understanding the following:

  1. Each row from an examples table corresponds to a "suite", but following the "suite:start" events, there are 4 repeated test:start events with a uid "undefined" - I'm not sure how these are to be generated or what they mean. There are also 2 more of these at the end of each suite.
{ event: 'test:start',
  cid: '0-0',
  uid: 'undefined',
  pending: false,
  parent: 'my scenario outline8',
  type: 'test',
  err: {},
  duration: 0,
  runner: 
   { '0-0': 
      { enableVNC: true,
        acceptSslCerts: true,
        loggingPrefs: [Object],
        browserName: 'chrome',
        chromeOptions: [Object],
        name: 'feature' } },
  specs: [ 'C:\\work\\folder\\src\\features\\feature.feature' ],
  tags: [],
  featureName: 'my feature',
  scenarioName: 'my scenario outline',
  specHash: '63b5a1ed148745c7f0c57d32206acbb8' }

2. The "suite:start" events don't mention which Examples row has generated this "suite" - there's no way to find the matching row except by counting repetitions.

3. There is no mention of example tables, and no way of knowing which example table a given test:start is referring to (so e.g. "Big things" is never mentioned)

4. For test cases which are generated from steps that don't contain example data, the "uid" is duplicated for each "suite", so for example "step 24" appears 4 times - once per Examples row.

I'm not sure if these things are by design, or required by the reporter, or if they should be improved. Do you know?

@BorisOsipov
Copy link
Contributor

@goofballLogic

there are 4 repeated test:start events with a uid "undefined" - I'm not sure how these are to be generated or what they mean.

it's cucumber or wdio hooks.
Cucumber also fires for hooks events pair "test-step-started\finished" but without references to Cucumber document lines. I tried to deal with it in https://github.com/webdriverio/wdio-cucumber-framework/pull/110/files but now I am not sure how it will work with your chages. I guess we can rework it later if it will be need.
don't pay attention to them here.

I've disabled hooks and update your jsfiddle

  1. The "suite:start" events don't mention which Examples row has generated this "suite" - there's no way to find the matching row except by counting repetitions.

I guess numbers in "suite:start" event.uid "my scenario outline8", "my scenario outline9" reffer to corresponding row in cucumber document?

There is no mention of example tables, and no way of knowing which example table a given test:start is referring to (so e.g. "Big things" is never mentioned)

yes. do we need this? I'm not sure.

@BorisOsipov
Copy link
Contributor

  1. For test cases which are generated from steps that don't contain example data, the "uid" is duplicated for each "suite", so for example "step 24" appears 4 times - once per Examples row.

It is ok. "test:start" event uids must be unique for corresponding suite only. am I right? @christian-bromann

@christian-bromann
Copy link
Contributor

It is ok. "test:start" event uids must be unique for corresponding suite only. am I right?

yes

@goofballLogic
Copy link
Contributor Author

@BorisOsipov do you have the link to your fork of my jsfiddle?

Also - I checked the default cucumber-js reporter (and popular formatter cucumber-pretty) and neither of them bother to mention the examples table names or row numbers, so I guess we don't need to either.

Note: There could be an opportunity to steal a march on cucumber-js here by providing better reporting through wdio than they do (could be a configurable option for wdio-cucumber-framework?)

@BorisOsipov
Copy link
Contributor

@goofballLogic
Copy link
Contributor Author

@BorisOsipov this is mostly working. Just a few problems now:

  • indentation
  • embedding example data into the step

(see below)
I think I'll also add some other samples so we get this right.

[phantomjs #0-0] Session ID: a7454530-5cde-11e8-b27d-9909f5a181aa
[phantomjs #0-0] Spec: C:\src\wdio-cucumber4-issue\passing.feature
[phantomjs #0-0] Running: phantomjs
[phantomjs #0-0]
[phantomjs #0-0] A passing feature
[phantomjs #0-0]
[phantomjs #0-0]     A passing scenario
[phantomjs #0-0]       √ I click the clickable region
[phantomjs #0-0]       √ I should get the result: <test>
[phantomjs #0-0]
[phantomjs #0-0]         A passing scenario
[phantomjs #0-0]           √ I click the clickable region
[phantomjs #0-0]           √ I should get the result: <test>
[phantomjs #0-0]
[phantomjs #0-0]
[phantomjs #0-0] 4 passing (1s)
[phantomjs #0-0]

@goofballLogic
Copy link
Contributor Author

@BorisOsipov / @christian-bromann ok I'm still banging my head against the wall with this.

The look-ups required to get the correct data for every case are quite byzantine. Have we already considered using Cucumber's built-in helpers for the "cucumberEventListener" instead of writing our own?

In particular, https://github.com/cucumber/cucumber-js/blob/master/src/formatter/helpers/event_data_collector.js would do most of the heavy lifting for us and is already exported from the cucumber package.

@BorisOsipov
Copy link
Contributor

Have we already considered using Cucumber's built-in helpers for the "cucumberEventListener" instead of writing our own?
It will be better to reuse cucumberjs helpers.

@christian-bromann christian-bromann mentioned this pull request May 27, 2018
@goofballLogic
Copy link
Contributor Author

goofballLogic commented Jun 1, 2018

@BorisOsipov I've been ill for a while and am just getting back to this.

Before I try to switch out such a key piece of the codebase (cucumberEventListener largely replaced by https://github.com/cucumber/cucumber-js/blob/master/src/formatter/helpers/event_data_collector.js) I'm trying to properly understand the purpose of the specs included in the current suite which I'll post here. Can you check whether I'm guessing the meaning correctly?

adapter.spec.js

Background

wdio-cucumber passes through many of the options supported natively by cucumber itself. In addition, wdio's concept of "compiler" is implemented. The default cucumber package is switched out for wdio's cucumber package prior to the (re)loading of users' feature/spec files so that both wdio's and user's code are sharing the same version and runtime instance. Steps are then wrapped to allow for wdio's "retry" semantic. Finally the cucumber steps are executed by calling the underlying cucumber library.

Purpose of this spec

The spec only tests the following:

  • compiler (e.g. js:babel-compiler) must be specified, otherwise the run is aborted with an Error
  • as part of the reloading of spec/feature files it should be possible to use globs

Other functionality is currently not covered by the spec.

cucumber-hooks.spec.js

Background

Running cucumber specs should include normal cucumber "before", "after", "beforeAll", "afterAll" steps.

Purpose of this spec

❓ The spec claims to test "Executes feature files with cucumber hooks" but I can't find any part of the spec code which is actually testing this. Instead the test succeeds if the cucumber spec runs end-to-end (regardless of whether cucumber hooks were executed). I'm not sure what this spec is actually testing. Help? ❓

error.handling.spec.js

Background

The somewhat mysterious title "ignores service hook errors" suggests that WebdriverIO is designed to fail silently on anything except the actual test steps themselves.

❓ Could someone confirm that this is in fact the desired way for WebdriverIO to work? ❓

Purpose of this spec

TBD

hooks.spec.js

Background

For each of wdio's built in hooks (before, beforeFeature, beforeScenario, beforeStep, beforeCommand, afterCommand, afterStep, afterScenario, afterFeature, after), code can be executed which which resolves using any of:

  • async function
  • promise (native or q)
  • browser command (wdio-native, or custom - where a custom step might comprise any of the others listed here)

The test runner must ensure that these steps complete before proceeding to run the subsequent steps.

Purpose of this spec

The spec verifies that for each combination identified above, the runner waits until the hook has correctly completed before continuing.
In the case of native promises only it also verifies that the:

  • hook is executed at all
  • before, after hooks "should contain capabilities and spec parameters" (two arguments passed)
  • beforeFeature, afterFeature hooks "should contain right feature data" (an argument passed)
  • beforeScenario, afterScenario hooks "should contain right scenario data" (an argument passed)
  • beforeStep, afterStep hooks "should contain right step data" (an argument passed)
  • beforeCommand hook "should contain right command parameter" (checks the first argument)
  • afterCommand hook "should contain right command parameter" (checks the first argument, plus the result in the second argument)

❓ N.B. I don't see these "before" or "after" hooks, but do see "beforeSuite" and "afterSuite" hooks listed here: http://webdriver.io/guide/getstarted/v4.html#Hook-me-up . Are "before" and "after" still valid? ❓

reporter.spec.js

Background

cucumber-js provides an event life-cycle comprising enough information to "allow us to build cross-platform formatters and other tools that process test results." (see cucumber/common#172 ). The intent of wdio-cucumber-framework is to tap this event stream and to produce a transformed stream of events capable of consumption by the wdio reporters (the base class for which contains a list of accepted events: https://github.com/webdriverio/webdriverio/blob/master/lib/utils/BaseReporter.js ).

The existing implementation of this event translation layer taps into the event lifecycle by passing an EventEmitter instance into the various cucumber-js methods adapted by adapter.js. Cucumber uses the provided event emitter to emit the cucumber event life-cycle events. This emitter's events are listened to by the CucumberReporter class which, in collaboration with the CucumberEventListener, generates (and/or stores for later generation) the required wdio events.

The cucumber-js also provides an EventDataCollector class, the purpose of which is to handle "the complexity of grouping the data for related events" (see https://github.com/cucumber/cucumber-js/blob/master/src/formatter/helpers/event_data_collector.js, https://github.com/cucumber/cucumber-js/blob/master/docs/custom_formatters.md#custom-formatters ). However, this class (and the associated gherkin_document_parser.js and pickle_parser.js) are not an official API and a custom method of grouping and parsing the event data has been built instead.

An exact mapping of cucumber event data to wdio event data is still being worked out as the initial implementation has proven inadequate in cases such as cucumber Background and Scenario Outline feature constructs. An overview of the existing implementation's mapping is provided here as a rough guideline rather than a correct, complete specification:

cucumber-js event wdio event
gherkin-document suite:start (tied to feature)
pickle-accepted suite:start (tied to scenario)
test-case-prepared - (steps normalised for use in later events)
test-step-started test:start
test-step-finished test:fail or test:pass or test:pending
test-case-finished suite:end (tied to scenario)
test-run-finished suite:end (tied to feature)

Purpose of this spec

The spec, as currently defined, attempts to take a static recording of events generated during a sample test run and to replay those events through the event-translation layer, inspecting the output to verify if the appropriate translation has occurred.

retry.spec.js

Background

As noted here: http://webdriver.io/guide/testrunner/retry.html#Rerun-Step-Definitions-in-Cucumber the intent of wdio is that you can retry failing specs up to a specified maximum number of times in an attempt to deal with "e.g. flaky network or race conditions"

Purpose of this spec

The spec verifies that each of the following types of step are successfully retried the expected number of times (and only the expected number of times) before completing successfully:

  • promise
  • wdio command
  • async function + wdio command

tests.spec.js

Background

wdio executes test code synchronously by virtue of delegating non-blocking calls to a Fiber environment. The test runner must ensure that both sync and async calls execute synchronously from the perspective of the test writer.

Purpose of this spec

❓ Somehow this spec is verifying that in a simulated condition whereby a browser can execute certain commands "synchronously" (i.e. they are awaitable) the tests correctly complete steps by waiting for the completion of a prior step. I am not sure how this test works as it appears to inspect a magic number referring to longer test execution time when Fiber environment is used

@goofballLogic
Copy link
Contributor Author

ok @christian-bromann , @BorisOsipov I've completed my analysis of the test suite. There are a couple of questions which you can see in the text above.

Thinking about the event lifecycle which cucumber-js uses, I note that it's somewhat in flux (e.g. see
cucumber/common#172 ).

The current approach for testing our reporting capability is to unit test the event translation layer based on a recording of events from running cucumber and seeing what it spits out. However this approach appears problematic in the following ways:

  1. The events recorded may change as cucumber updates itself, invalidating our mapping layer
  2. We would need to handle more sequences of events than we currently handle (i.e. one recording is not enough), due to the complexities surrounding things like Background and Scenario Outline.

One potential way to mitigate the above is to take a more outside-in approach and simply have a batch of cucumber feature files for which we know the desired stream of wdio events, and do a test integrated with cucumber itself. This would allow us to implement code which is more agnostic about the exact events being raised, especially if we leverage the EventDataCollector class in cucumber to do the "description" part of the problem for us. (although note that EventDataCollector isn't really a "published" API either...)

If this approach works for you, could I suggest that we start by compiling a set of feature files (I think 5 should be enough), along with the desired wdio events output (@BorisOsipov you seem to know this aspect?) and I can have a go at a new translation layer.

@christian-bromann
Copy link
Contributor

@goofballLogic thanks for putting this together. I will get back to you as soon as possible

@goofballLogic
Copy link
Contributor Author

@christian-bromann , @BorisOsipov if you guys can't get availability to look into the stuff I supplied above, another alternative would be for me to rebuild the library entirely which might be quicker. The trouble with the existing code is that I'm not entirely sure of the intent being communicated by the existing code and tests.

The rebuild option would require you to supply (or approve my guessing about) the following:

  1. A set of Cucumber feature files and the resulting reports that should be generated
  2. "mixed functionality" (such as supporting wdio-style hooks alongside Cucumber-style hooks)
  3. CLI options which must be supported by the library

@christian-bromann
Copy link
Contributor

@goofballLogic would you mind joining the v5 Gitter channel to discuss details? I would love to get the framework ported to v5 with the new behavior you described above. I am not using Cucumber on daily basis and don't have a lot experience of the lifecycle. For WebdriverIO it is important that certain events are propagated upstream so our reporters work appropriately. We disabled native reporter because they don't make much sense in the e2e world imho. That is why we have custom wdio reporter that include information on capabilities and suites. I am open to change this in the next major release.

@wswebcreation
Copy link
Contributor

Hi Christian

Why wouldn't the native reporter for Cucumber make sense?
I think Webdriver.io is now doing double work and reporters now need to "rebuild" the report to get a proper/consistent result?
(Just curious 😉)

@moreirasantos
Copy link

Hey wswebcreation,

because the output would be all jumbled up otherwise (parallel testing). Have a look a this:
#6

@wswebcreation
Copy link
Contributor

I've seen that one yeah, but the cucumber framework of Protractor has a different approach

https://github.com/protractor-cucumber-framework/protractor-cucumber-framework

It works without to many problems and also a very small module. Clould this also be an approach?

@goofballLogic
Copy link
Contributor Author

@wswebcreation yes this is the type of integration I had envisaged. It's slightly more complicated because of integrating things like wdio native hooks, but in general that's how I reckon it should be done.

@wswebcreation
Copy link
Contributor

wswebcreation commented Jul 5, 2018

@goofballLogic
Ahh, cool, do you still want to create your own reporter or do you want to rely on the CucumberJs reporter?

By the way, I've created this PR to add some more data to the current reporter (#133). Will that still be needed then?

@goofballLogic
Copy link
Contributor Author

goofballLogic commented Jul 5, 2018 via email

@goofballLogic
Copy link
Contributor Author

@christian-bromann I have done so

@christian-bromann
Copy link
Contributor

@goofballLogic I am open to any changes as long as they are aligned with the other frameworks meaning that wdio reporter are still supported and commands can be run synchronously. I haven't ported this framework to v5 so this might be a chance to start on a green field with this integration. What do you think?

@goofballLogic
Copy link
Contributor Author

@christian-bromann yes green-field is best i think

@christian-bromann
Copy link
Contributor

@goofballLogic ok, so to make it work with the wdio testrunner there are the following requirements:

  • test state events are passed along (e.g. test:start etc.) so that wdio reporter work
  • step definitions are automatically wrapped so that commands in there are executed synchronously
  • the following hooks need to get triggered: before, beforeSuite, beforeTest, beforeCommand, afterCommand, afterTest, afterSuite, after
  • I guess it would be also great to move the "new" integration to the v5 repository so it can be published along the other modules

I would be happy to add you to the project committers of v5 or you can just build an integration that works with v4 and we will move it to v5 when the it is stable.

@christian-bromann
Copy link
Contributor

Why wouldn't the native reporter for Cucumber make sense?

@wswebcreation just to enable some better integration into wdio and the e2e space. Cucumber (or other test framework) reporters are focused on unit testing. They don't have information on which browser is running or about the session id of a test. As @miguelsantos500 mentioned parallel testing is also a big factor. You want to have reports for each individual session and test not everything mixed together. If I would have decided to go with native reporters I would have written a wdio flavor of them anyway so I decided to let wdio have its own reporting system.

@goofballLogic
Copy link
Contributor Author

Dear Cucumberites,

To try and fix the Cucumber integration I've created a "sub package" within the new v5 wdio framework. Work can be found at /packages/wdio-cucumber-framework in my fork of v5. The integration will hopefully be released with v5 of wdio ("in beta for the next month or two at least")

This will be a new adapter and I'm starting with some design work to try and get things right. An initial design doc brainstorming functionality that needs to be tested can be found here: /packages/wdio-cucumber-framework/docs/design/important-features.md.

There are a number of outstanding questions (see Answer pending). I'd welcome help thinking it through (including PRs for my fork to answer questions or add documentation).

@goofballLogic
Copy link
Contributor Author

Closing this PR as it doesn't fully fix the problem. Work on wdio v5 implementation of the cucumber reporter is now my priority.

@goofballLogic
Copy link
Contributor Author

The code is still available if anyone else wishes to resubmit the PR for wdio v4. Happy to explain where I got to with this PR if it helps...

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.

5 participants