Option to "fail fast" on BeforeFeatures catastrophic failure

[yaronassa]

While I don't want to use the --fail-fast behaviour (i.e., I want all the tests that could run, to run, regardless of previous failures), I do want to "fail fast" if the BeforeFeatures hook fails (since that means something very fundamental in my env. setup has failed).

A quick and dirty way to achieve a similar practical result could be for BeforeFeatures to be able to flip the --fail-fast switch, so that the first step run will be the last. A better solution would be for a promise rejection from that hook to immediately fail the run, regardless of the fail fast flag.

What do you think?

[charlierudolph]

The BeforeFeatures event handler should already do this

[yaronassa]

I examined this behavior, and I now understand that my problem actually stems from
#686, which have been resolved in 2.0

Thanks for the prompt reply.

[yaronassa]

Upon further investigation it seems the when the beforeFeatures hook fails, the CLI exists, but with exit code = 0 (I expected it to be 1)

[charlierudolph]

We have tests that the build fails when a handler errors. What version are you using and can you please provide the minimal code for a reproducible example?

[yaronassa]

Cucumber 1.2.1
Node 6

I've managed to narrow down the problem to the following single change.
Seems that when a rejected promise is returned from the hook (instead of a simply thrown error), the rejection isn't handled. This leads the hook to know that an error had occurred and stop the run, but the process exists with code = 0.

BeforeFeatures hook throws an error:

module.exports = function () { 
    this.registerHandler('BeforeFeatures', function(){
        throw new Error('Something bad');
    });
};

• No features are run
• The process exits with code = 1

BeforeFeatures hook returns a rejected promise

module.exports = function () { 
    this.registerHandler('BeforeFeatures', function(){
       return Promise.reject(new Error('Something bad'));
    });
};

OR

module.exports = function () { 
    this.registerHandler('BeforeFeatures', function(){
        return Promise.resolve().then(() => {
            throw new Error('Something bad');
        });
    });
};

• No features are run
• The process exits with code = 0
• Node / bluebird throws an unhandled rejection warning: (node:1024) UnhandledPromiseRejectionWarning: Unhandled promise rejection

[yaronassa]

Hmmm. Actually now that I know what it stems from, it seems to be covered in #708, which was resolved in 2.0.

Is that the case?

[charlierudolph]

Uncertain. That issue mentions that it works as expected in 1.3.1. That issue got raised due to a change in how the code in built in 2.0. Thus that issue was 2.0 specific.

[yaronassa]

Seems to be caused by throwing the error synchronically in EventBroadcaster.broadcastEvent listener callback. I guess that since the error originated within a promise, it's swallowed by the promise rejection which then becomes unhandled.

Just to test things, I changed asyncForEach to return the last listener error in its callback, propagated the error back from broadcastEvent to broadcastAroundEvent and threw it there before calling userFunction. This resulted in the correct behavior.

I don't presume to know enough about cucumber's internals and error propagation to say this should be the solution (it seems arbitrary, and for all I know might break everything).

Just sharing my findings hoping that it would lead to the real solution.

[charlierudolph]

Any chance you could open a PR with your changes? It would be against the "1.x" branch. The PR will run all the tests and we can see its effect.

[yaronassa]

Sure.

[floribon]

I would add that there is always the possibility to process.exit(2) if something very wrong happened, which is what we do.

[yaronassa]

Sorry for the confusion with the first PR.
My bad.

The solution I committed is less radical than the changes I specified here. I simply throw the error within process.nextTick so it won't be in the promise context. This solution has no consequences on the calling code.

[charlierudolph]

@yaronassa Just released 1.3.3 with this fix. Sorry that took so long.

[yaronassa]

@charlierudolph
Awesome, thanks

[lock]

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.