-
Notifications
You must be signed in to change notification settings - Fork 60
Fix support for Scenario Outlines #126
Fix support for Scenario Outlines #126
Conversation
@goofballLogic can you rebase on top of current master? I already merged your previous PR. |
Related to #111 |
@goofballLogic can you do: $ git rebase origin master There seems to be rebase conflicts and I would like to avoid a merge commit. |
nevermind, I can also just squash it |
@goofballLogic it still fails on my example.
|
I guess the problem that
|
@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... |
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:
The hierarchy is:
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:
the hierarchy is:
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? |
@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
if I understood your idea is the same as old behaviour. Hope it helps you with this trickly issue. |
Thanks I'll try to match this
Sent from TypeApp
…On 18 May 2018, 21:40, at 21:40, Boris Osipov ***@***.***> wrote:
@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.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#126 (comment)
|
@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:
|
it's cucumber or wdio hooks. I've disabled hooks and update your jsfiddle
I guess numbers in "suite:start" event.uid "my scenario outline8", "my scenario outline9" reffer to corresponding row in cucumber document?
yes. do we need this? I'm not sure. |
It is ok. "test:start" event uids must be unique for corresponding suite only. am I right? @christian-bromann |
yes |
@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.
|
@BorisOsipov this is mostly working. Just a few problems now:
(see below)
|
@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 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.jsBackgroundwdio-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 specThe spec only tests the following:
Other functionality is currently not covered by the spec. cucumber-hooks.spec.jsBackgroundRunning 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.jsBackgroundThe 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 specTBD hooks.spec.jsBackgroundFor 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:
The test runner must ensure that these steps complete before proceeding to run the subsequent steps. Purpose of this specThe spec verifies that for each combination identified above, the runner waits until the hook has correctly completed before continuing.
❓ 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.jsBackgroundcucumber-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:
Purpose of this specThe 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.jsBackgroundAs 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 specThe 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:
tests.spec.jsBackgroundwdio 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 ❓ |
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 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:
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. |
@goofballLogic thanks for putting this together. I will get back to you as soon as possible |
@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:
|
@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. |
Hi Christian Why wouldn't the native reporter for Cucumber make sense? |
Hey wswebcreation, because the output would be all jumbled up otherwise (parallel testing). Have a look a this: |
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? |
@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. |
@goofballLogic By the way, I've created this PR to add some more data to the current reporter (#133). Will that still be needed then? |
My hope was an adapter (as light as possible)
Envoyé par TypeApp
Le 5 juil. 2018 à 18:43, à 18:43, Wim Selles <notifications@github.com> a écrit:
…
@goofballLogic
Ahh, cool, do you still want to create your own reporter or do you want
to rely on the CucumberJs reporter?
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#126 (comment)
|
@christian-bromann I have done so |
@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? |
@christian-bromann yes green-field is best i think |
@goofballLogic ok, so to make it work with the wdio testrunner there are the following requirements:
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. |
@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. |
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). |
Closing this PR as it doesn't fully fix the problem. Work on wdio v5 implementation of the cucumber reporter is now my priority. |
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... |
Scenario outlines require parsing the gherkin document's Examples hash in addition to the Steps hash to determine the location of an executing step