fix: short-circuit execution for skipScenario in StepExecutionStage a…

[serhatozdursun]

Properly Skip Scenarios When SkipScenarioException Is Thrown in Hooks

This PR addresses issue #926:

Throwing SkipScenarioException in a @BeforeScenario hook does not mark the scenario as skipped as expected.

---

Summary of Changes

• Short-circuit execution for skipped scenarios:
  • Updated StepExecutionStage and HookExecutionStage to check for skipScenario in the previous stage result, mirroring the handling of failures. If a scenario is marked as skipped, subsequent stages are not executed.
• Test coverage:
  • Added/updated tests in StepExecutionStageTest and created HookExecutionStageTest to verify that execution is properly short-circuited when skipScenario is set.
• Behavior:
  • Now, when SkipScenarioException is thrown in a hook (e.g., @BeforeScenario), the scenario is correctly marked as skipped and its steps are not executed. This ensures the Gauge summary and reports reflect the correct skip count, as expected by users.

---

How to Reproduce / Test

1. Use a @BeforeScenario(tags = "skip") hook that throws SkipScenarioException.
2. Run a spec with a scenario tagged skip.
3. The scenario should be reported as skipped, and its steps should not run.

---

References

• Fixes: #926

it will look like this:

[serhatozdursun]

@chadlwilson hey once you have time, please can you take look and please let me know if any change needed

[chadlwilson]

Sorry, I've been focused elsewhere and haven't had a chance to get to this. Takes a bit longer as im not deeply familiar with the code.

[serhatozdursun]

@chadlwilson No problem at all. I tested it locally and can confirm the issue—he’s already applied the fix with this PR.

Note: Please feel free to assign me other tasks or bug fixes in the Java package. I’m getting more familiar with the code and would be happy to help further.

[chadlwilson]

I don't really understand why there'd be the need to change the behaviour for failure in HookExecutionStage here. That'd imply something was broken for failures in hook processing earlier, which has not been reported?

Something doesn't feel correct about this, and it makes me feel I don't understand something about the role of the HookExecutionStage vs HooksExecutor, and when results need to be merged vs not merged. Unfortunately I don't really have time/energy to test through all of the scenarios or review if they are covered by the existing functional tests at https://github.com/getgauge/gauge-tests .

The questions I have

• If the handling for failure needed to be changed in HookExecutionStage - why was this, and what was the consequence earlier?
• Why did StepExecutionStage need changing as well? Does this imply a different issue with regular steps not described in Cannot dynamically skip scenario during BeforeScenario hook #926?
• Why do you feel changing HookExecutionStage is the correct place, rather than HooksExecutor?
• What happens if you have multiple @BeforeScenario hooks defined?
  • Say you have 3 - 1 passes, 1 fails and 1 implies a skip of the scenario? Does it matter how are they batched together, or spread among executors and what is reported in the result? Does it seem correct?

[chadlwilson]

By chance, was a large chunk of that response auto-generated by AI? It doesn't really get to the deeper points, several bits don't make sense in context, and the answer seems to directly answer a points without adequate context or correlation to why the changes were needed.

• Why did StepExecutionStage need changing as well? Is this different from Cannot dynamically skip scenario during BeforeScenario hook #926?

  • Same gap existed at the step stage: subsequent steps could still run after a skip. We added the same short‑circuit. This directly addresses Cannot dynamically skip scenario during BeforeScenario hook #926.

This doesn't make sense, given the issue #926 was specifically about BeforeScenario, not regular steps (which this code changes). if this comment is indeed true, it implies the very core functionality intended by #924 doesn't actually work, which would mean the whole thing should be reverted.

[serhatozdursun]

Yes I was prepared only the answer text with AI but during the test in my local I have noticed an issue. now will update the PR but this time expnation will from no worries :)) sorry about the recent one as I am a little bit busy just wanted to be quick :) but relaying on AI was mistake again sorry

[serhatozdursun]

If the handling for failure needed to be changed in HookExecutionStage - why was this, and what was the consequence earlier?

The handling for failure in HookExecutionStage needed to be changed because it was skipping the run but not marking the scenario as skipped. And this marked issue also was exists on step level.

Why did StepExecutionStage need changing as well? Does this imply a different issue with regular steps not described in #926?

I followed the getFailed() method because while skipping tests was working, the scenarios weren’t being marked as skipped. Since getFailed() correctly marks a scenario as failed, I thought replicating that logic would allow me to mark skipped scenarios properly too. Later, I realized I had missed the main piece in the BeforeScenario hook. I’ve now pushed that change as well.

Why do you feel changing HookExecutionStage is the correct place, rather than HooksExecutor?
Answerd above

What happens if you have multiple @BeforeScenario hooks defined?
* Say you have 3 - 1 passes, 1 fails and 1 implies a skip of the scenario? Does it matter how are they batched together, or spread among executors and what is reported in the result? Does it seem correct?
Say you have 3 - 1 passes, 1 fails and 1 implies a skip of the scenario: -> it would like the following

the code was like

import com.thoughtworks.gauge.*;

import java.util.HashSet;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;

public class StepImplementation {

    private HashSet<Character> vowels;

    @BeforeScenario(tags = "skip")
    public void skipScenario(ExecutionContext context) {
        System.out.println("skip " + context.getCurrentScenario().getName() + "...");
        Gauge.writeMessage("Tagged to skip");
        throw new SkipScenarioException("Tagged to skip");
    }

    @BeforeScenario()
    public void beforeHook(ExecutionContext context) {
        Gauge.writeMessage("Before Scenario 2");
    }

    @Step("Vowels in English language are <vowelString>.")
    public void setLanguageVowels(String vowelString) {
        vowels = new HashSet<>();
        for (char ch : vowelString.toCharArray()) {
            vowels.add(ch);
        }
    }

    @Step("The word <word> has <expectedCount> vowels.")
    public void verifyVowelsCountInWord(String word, int expectedCount) {
        int actualCount = countVowels(word);
        assertThat(expectedCount).isEqualTo(actualCount);
    }

    @Step("Almost all words have vowels <wordsTable>")
    public void verifyVowelsCountInMultipleWords(Table wordsTable) {
        for (TableRow row : wordsTable.getTableRows()) {
            String word = row.getCell("Word");
            int expectedCount = Integer.parseInt(row.getCell("Vowel Count"));
            int actualCount = countVowels(word);

            assertThat(expectedCount).isEqualTo(actualCount);
        }
    }

    @Step("Skip the test <string>")
    public void skipTest(String string) {
        throw new SkipScenarioException("Tagged to skip");
    }

    @Step("Passing Step")
    public void notSkipTest() {
    }

    @Step("Falling Step")
    public void failStep() {
        fail("Falling Step");
    }

    private int countVowels(String word) {
        int count = 0;
        for (char ch : word.toCharArray()) {
            if (vowels.contains(ch)) {
                count++;
            }
        }
        return count;
    }
}

and spec file was like

Specification Heading
=====================

This is an executable specification file. This file follows markdown syntax.
Every heading in this file denotes a scenario. Every bulleted point denotes a step.

To execute this specification, run
	gauge specs


* Vowels in English language are "aeiou".

Vowel counts in single word
---------------------------

tags: single word

* The word "gauge" has "4" vowels.


Vowel counts in multiple word
-----------------------------

This is the second scenario in this specification

Here's a step that takes a table

* Almost all words have vowels
     |Word  |Vowel Count|
     |------|-----------|
     |Gauge |3          |
     |Mingle|2          |
     |Snap  |1          |
     |GoCD  |1          |
     |Rhythm|0          |

skipping scenario
------------
tags: skip
* Falling Step

For multiple Before hook

I just added the following BeforeScenario code the above shared code

    @BeforeScenario()
    public void beforeHook(ExecutionContext context) {
        Gauge.writeMessage("Before Scenario 2");
    }

Normally, there should be two failing scenarios: one from * The word "gauge" has "4" vowels. and another from * Falling Step. Since we tagged skipping scenarios with skip and implemented a BeforeScenario hook to handle them, the code is now skipping those scenarios as intended and generating the report correctly.