Fixing issue #138

[Rob-Meijeren]

This is the fix for issue #138
By checking in the getStepFromFeature function if we are processing a scenario and if so if we have the currently executing scenario the difference with the 2.2.0 version that appeared is gone.

I have checked this with the cucumber-boilerplate project as well as my personal project that it works

[Rob-Meijeren]

There is actually failures in the travis build. Will check those. Forgot to run the tests before I committed 😒

[christian-bromann]

@Rob-Meijeren can you add the following change? From:

    "test:unit": "mocha --compilers js:babel-core/register \"./test/!(adapter.spec).js\"",
    "test:postadapter": "mocha test/adapter.spec.js",

to:

    "test:unit": "mocha --compilers js:babel-core/register \"./test/!(adapter.spec).js\" && mocha test/adapter.spec.js",

[Rob-Meijeren]

@christian-bromann Sure I can. Do you also want to completely remove the test:postadapter script? or just combine the 2 in test:unit and let the test:postadapter script stay where it is (but obviously remove it from the test script as else it would get executed twice)

Also I ran into something strange. After looking why the first test is failing (the one from cucumber-hooks.spec.js) I saw that the index that is coming from the testStepStartedEvent in onTestStepStarted is different than what I would get back as to what I get running it in the cucumber-boilerplate project.

When I ran the tests in the cucumber-boilerplate and logged the index I would get a good index number (e.g. 0 1 2 when there would be one step in the background part and 2 steps in the scenario) for every scenario but when I run the test from cucumber-hooks.spec.js I would get 0 1 2 3 when any combination of background and scenario would not have more than 3 steps and should give 0 1 2.

How and why this is happening I truly have no clue. Could you take a look into this?

The reporting is working correct in the cucumber-boilerplate and my own project when I keep it as is in the commit of this pull request but the test fails.

The other test that failed was because of the placeholder in the step text of the scenario outline is not replaced and it was expecting the filled in text which I changed like the others from the previous pull request

[christian-bromann]

How and why this is happening I truly have no clue. Could you take a look into this?

I am not familiar with Cucumber. Can't we just generate a random number? Maybe the person who raised the issue used a wrong cucumber-js version.

[Rob-Meijeren]

Well the cucumber-boilerplate project itself doesn't have a dependency on cucumber in there so it takes the one from this project which is currently 4.2.1 and checking the node_modules on both also shows that definitly both project are having the same cucumber version.

[Rob-Meijeren]

regarding the random number it can't be random as it will point to the current step of the scenario that is being executed. If this would be random the text send to the reporter would also be random and it would solve the issue that this PR was supposed to solve 😅

[Rob-Meijeren]

also it is coming from the cucumber runtime (as far as I can see) so I wouldn't know to give the random number to the event at that point

[Rob-Meijeren]

@christian-bromann I have found the issue this morning after some deep diving in webdriverio. It turned out that it was failing because of hooks. After I changed the onTestCasePrepared function the hooks were no longer attached to the scenario which caused the offset. the cucumber-boilerplate would go correct because there were no hooks there.

Before I commit I wanted to ask you one thing though. You asked me to change the scripts in the package.json. But do you want to remove the test:postadapter script all together or just paste the same command behind the test:unit script?

[christian-bromann]

So the issue with the setup right now is that if the unit tests fail but the postadapter tests pass the whole build passed. Let's still run the postadapter tests but in a way that the unit test can cause the build to fail. I think if you do the change I explained above it will work like that.

[Rob-Meijeren]

@christian-bromann I can confirm that the change you proposed works as expected however I would prefer to keep the scripts apart and not mingle them just to fail the build when neccesary. We could also use the bail flag for mocha. I just tested it and we can keep the scripts as is only the test:unit test would change from "test:unit": "mocha --compilers js:babel-core/register \"./test/!(adapter.spec).js\"" to
"test:unit": "mocha --bail --compilers js:babel-core/register \"./test/!(adapter.spec).js\""

[christian-bromann]

I am not sure why the --bail parameter would change this behavior. The build is passing because the adapter test is run after the unit test and it looks like that the result of the last task is determine whether the build fails or not.

[Rob-Meijeren]

well not exactly. It looks like is a bug in mocha because of which the exit code it not set properly (mochajs/mocha#2713) when using the --bail flag the exit code is properly set and since we do a run-s the build and postadapter would only run after the unit tests but when the unit tests fail with the proper exit code the whole process exits causing the build to fail

[christian-bromann]

@Rob-Meijeren ok, I gonna trust you on that one

[Rob-Meijeren]

@christian-bromann The changes were pushed and I just followed the build to be sure and it passes

[christian-bromann]

👍