cucumber / common

A home for issues that are common to multiple cucumber repositories
https://cucumber.io/docs
MIT License
3.36k stars 694 forks source link

allow more steps to execute after step failure #2161

Open mrsenzy opened 8 months ago

mrsenzy commented 8 months ago

🤔 What's the problem you're trying to solve?

Playwright soft assertions are not working with cucumber runner. They work well with playwright runner.

✨ What's your proposed solution?

Looking for option to capture error and set status manually as Failed when soft exception occurs and continue the testing steps.

⛏ Have you considered any alternatives or workarounds?

Tried by capturing the exception and print at end of After hooks and fail test there but still not satisfier compared with soft assertion which fails the test step but still continues the test. Tried custom method for capturing error and assigning test step status as failure but current cucumber library doesn't allow that

📚 Any additional context?

Please let me know if there alternate option available within cucumber to fail step for soft assert type error and still continue execution. If we are able to update the error programmatically and still continue the process will also help.


This text was originally generated from a template, then edited by hand. You can modify the template here.

davidjgoss commented 8 months ago

I moved this to the common repo because I think it's a broader thing.

There's no mechanism to retroactively amend the result status of a step, and I don't think this is something we would do anyway because it would break the contract with formatters that consume results as they happen and expect them to be immutable.

@mattwynne @mpkorstanje @luke-hill has this handling of soft assertions come up before in other flavours?

mpkorstanje commented 8 months ago

It is sometimes requested. Though for most, making the soft assertions hard after the scenario is done seems to suffice.Looking at the essence of the problem I'm seeing the following requirements:

And the last one is the essence of the request.


Unlike Playwright Cucumber doesn't tightly integrate its own assertion library so we can't just collect the problems after the test step, mark it as failed and continue.

For pending steps we use a dedicated pending exception. But we can't really tell the difference between an exception thrown by making a soft assertion hard, and a regular assertion. Or for that matter, soft assertions made hard with the purpose of failing the step. So we'd need a mechanism to mark certain exceptions as "soft".

Let's suppose we have the following step definitions for Java using AssertJs SoftAssertions:

public class StepDefinitions {
    SoftAssertions softly = new SoftAssertions();

    @Given("I have {int} cukes in my belly")
    public void I_have_cukes_in_my_belly(int cukes) {
      // stuff
    }

    @When("I wait {int} hour")
    public void i_wait_hour(Integer int1) {
      Duration waited = /* stuff */;
      softly.assertThat(waited).isCloseTo(ofHours(1), ofSeconds(10));  
    }

    @Then("my belly should growl")
    public void my_belly_should_growl() throws InterruptedException {
        // stuff
    }
}

Wrapping with a dedicated exception

    @AfterStep
    public void assertSoftly(){
        Cucumber.throwSoftly(() -> softly.assertAll());
    }

By using throwSoftly we any exceptions thrown are wrapped in a CucumberSoftAssertionFailedError. Cucumber can recognize this exception and much like the PendingException treat it differently.

Marking a hook or steps as failing softly

    @AfterStep
    @ContinueOn(AssertionFailedError.class)
    public void assertSoftly(){
        softly.assertAll();
    }

Same idea as above, but now Cucumber will treat any AssertionFailedError thrown from a hook as a soft failure and continue execution.

    @When(value = "I wait {int} hour")
    @ContinueOn(AssertionFailedError.class)
    public void i_wait_hour(Integer int1) {
      Duration waited = /* stuff */;
      assertThat(waited).isCloseTo(ofHours(1), ofSeconds(10));
    }

And that could also be done for whole steps.

    @When(value = "I wait {int} hour")
    @ContinueOn(AssertionFailedError.class)
    public void i_wait_hour(Integer int1) {
      Duration waited = /* stuff */;
      assertAll(
              () -> assertThat(waited).isCloseTo(ofHours(1), ofSeconds(10)),
              () -> /* something else */
      );
    }

To continue on and test multiple assertions, use assertAll to assert multiple things at once. This avoids the need to even use a soft assertion library at all.

luke-hill commented 8 months ago

The first time I heard about soft assertions was in 2013 with selenium IDE (Legacy), where you had assert vs verify and other derivations.

Imo the best way to solve this would be in your assertion library, switching that to just doing something like

if CustomLibrary.new(assertion_logic).fail?
  error_chuck_into_log
end
mpkorstanje commented 8 months ago

@luke-hill that means the exceptions won't show up in any of the cucumber reports though. Unless Ruby works differently, in that case I'd like to know how. :smile:

davidjgoss commented 8 months ago

Report the failure in the associated step...Continue to the next step anyway...is the essence of the request.

Agreed. The actual mechanics of it - a certain type of exception, extra support code or config, etc - would be down to the implementation per platform norms and probably not too hard. But my thought is more, are we okay with marking a step failed and then doing more steps within a test case attempt? If so, do we need anything extra in messages? I'm leaning to yes and no respectively - I don't think this breaks any contract per se.

mrsenzy commented 8 months ago

For now we have a workaround logic by capturing the error using try catch and storing it in Error array and doing an assertion at @After to check if array has more than 0 and displaying the captured errors.

By this way the error is captured, step get passed and continue and fail is reported at after. Cucumber report is not looking good by this way as it shows step as passed inspite of error captured and reporting them at after hooks. couldnt refer to which step the error relates to as well as pickleStep from ITestStepHookParameter are available only at AfterStep

public async softAssert(customWorld: CustomWorld, condition: any, expectedVal: any, message?: string): Promise<any> {
    let data: any
    try {
        await condition(expectedVal);
        console.log("Soft Assert Message: " + message);
    } catch (error) {
        console.log(error.message.toString());
        data = 
            {
                "error": error.message.toString(),
            }
        customWorld.softAsserts.push(data);
    }
    return data;
}

After this.softAsserts.forEach(element => { this.attach(element.error, 'text/plain'); });

expect(this.softAsserts.length).toEqual(0)
mpkorstanje commented 8 months ago

But my thought is more, are we okay with marking a step failed and then doing more steps within a test case attempt? If so, do we need anything extra in messages? I'm leaning to yes and no respectively - I don't think this breaks any contract per se.

I agree with the conclusion but it is definitely a breaking change. Tools that collate steps into test cases will not have to deal with the possibility that there are multiple failing steps(i.e. multiple TestStepFinished.result.error messages). The cucumber-junit-xml-formatter comes to mind.

mattwynne commented 8 months ago

My suggestion is that we create a new step result status of warning. We can have (platform-specific) mechanisms, similar to what Rien suggests above, to flag some exceptions as mere warnings rather than outright failures.

A warning would not cause a scenario to abort execution immediately, but it would cause it to fail in the end. Or we could configure this as some kind of soft mode, vs a strict mode where it does abort immediately even on a warning.

I don't see why we couldn't make pending be a special case of warning - I've often found it irritating that I can't see beyond my first pending step when I'm sketching out step definitions, so this could actually be useful.

I agree that the implications of this change are breaking and a bit hard to predict. I suggest we run some kind of exploratory spike in cucumber-js before we commit to adding it to the message protocol etc.

davidjgoss commented 7 months ago

I'm not sure I agree re warning, that makes it sound innocuous - this would definitely still represent failure, and always cause the scenario to fail, but it would just allow more steps to execute to discover more.

At any rate, I think we're agreed across the core team that a first step would be to experiment in cucumber-js with allowing this - without a particular focus on the mechanism and interface for doing so - and see what effects there would be on formatters and plugins.

I'm leaving it unassigned though as it's unlikely to be a high enough priority to get serious attention in the short term. We can direct people with similar asks here and start to gauge the demand.