Skip to content

Commit

Permalink
Change Step FAILED/FINISHED Behavior to be more consistent when error…
Browse files Browse the repository at this point in the history 
…s are caught and not re-thrown (#3604)

* make failed step behavior consistent even if exception is caught and not rethrown.

* adapted tests

* fix step behavior for tests
  • Loading branch information
@jacob-wieland-gematik
jacob-wieland-gematik authored Jan 24, 2025
1 parent 8d55694 commit 992b79d
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 30 deletions.
11 changes: 1 addition & 10 deletions serenity-core/src/main/java/net/thucydides/core/steps/BaseStepListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import net.serenitybdd.annotations.TestAnnotations;
import net.thucydides.core.junit.SerenityJUnitTestCase;
import net.thucydides.model.domain.*;
import net.thucydides.model.domain.failures.FailureAnalysis;
import net.thucydides.model.domain.stacktrace.FailureCause;
import net.thucydides.model.screenshots.ScreenshotAndHtmlSource;
import net.thucydides.core.steps.session.TestSession;
Expand Down Expand Up @@ -905,8 +904,6 @@ private void markCurrentStepAs(final TestResult result) {
);
}

FailureAnalysis failureAnalysis = new FailureAnalysis();

public void stepFailed(StepFailure failure) {

if (!aStepHasFailed() || StepEventBus.getEventBus().softAssertsActive()) {
Expand All @@ -918,9 +915,6 @@ public void stepFailed(StepFailure failure) {

recordFailureDetails(failure);
}

// In all cases, mark the step as done with the appropriate result
currentStepDone(failureAnalysis.resultFor(failure));
}

public void stepFailed(StepFailure failure,
Expand All @@ -937,8 +931,6 @@ public void stepFailed(StepFailure failure,

recordFailureDetails(failure);
}
// Step marked as done with the appropriate result before
currentStepDone(failureAnalysis.resultFor(failure), timestamp);
}


Expand All @@ -951,8 +943,7 @@ public void stepFailedWithException(Throwable failure) {

recordFailureDetails(failure);
}
currentStepDone(failureAnalysis.resultFor(failure));
}
}

public void lastStepFailed(StepFailure failure) {
takeEndOfStepScreenshotFor(FAILURE);
Expand Down
2 changes: 0 additions & 2 deletions serenity-core/src/main/java/net/thucydides/core/steps/StepEventBus.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,6 @@ private void stepDone() {

public void stepFailed(final StepFailure failure) {

stepDone();
getResultTally().logFailure(failure);

for (StepListener stepListener : getAllListeners()) {
Expand All @@ -628,7 +627,6 @@ public void stepFailed(final StepFailure failure,
boolean isInDataDrivenTest,
ZonedDateTime timestamp) {

stepDone();
getResultTally().logFailure(failure);

for (StepListener stepListener : getAllListeners()) {
Expand Down
8 changes: 6 additions & 2 deletions serenity-core/src/main/java/net/thucydides/core/steps/StepInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,8 +495,12 @@ private void logStepFailure(Object object, Method method, Object[] args, Throwab
if (StepEventBus.getEventBus().aStepInTheCurrentTestHasFailed() && !StepEventBus.getEventBus().softAssertsActive()) {
return;
}
notifyOfStepFailure(object, method, args, assertionError);
LOGGER.debug("STEP FAILED: {} - {}", StepName.fromStepAnnotationIn(method).orElse(method.getName()), assertionError.getMessage());
try {
LOGGER.debug("STEP FAILED: {} - {}", StepName.fromStepAnnotationIn(method).orElse(method.getName()), assertionError.getMessage());
notifyOfStepFailure(object, method, args, assertionError);
} finally {
notifyStepFinishedFor(method, args);
}
}

private Object executeTestStepMethod(Object obj, Method method, Object[] args, Method zuperMethod, Object result) throws Throwable {
Expand Down
1 change: 0 additions & 1 deletion serenity-core/src/test/java/net/thucydides/core/steps/ConsoleStepListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ public void stepFinished() {
}

public void stepFailed(StepFailure failure) {
pop();
writeIndent(buffer);
buffer.append("--> STEP FAILED").append("\n");
}
Expand Down
3 changes: 2 additions & 1 deletion .../src/test/java/net/thucydides/core/steps/WhenRunningNonWebStepsThroughAScenarioProxy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,8 @@ public void the_proxy_should_skip_tests_after_a_failure() {
"-step1\n" +
"---> STEP DONE\n" +
"-failing_step\n" +
"---> STEP FAILED\n" +
"----> STEP FAILED\n" +
"---> STEP DONE\n" +
"-step3\n" +
"---> STEP IGNORED\n";
assertThat(listener.toString(), containsString(expectedExecution));
Expand Down
17 changes: 11 additions & 6 deletions serenity-core/src/test/java/net/thucydides/core/steps/WhenUsingTheStepEventBus.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,8 @@ public void should_record_step_failures() {
+ "-step1\n"
+ "---> STEP DONE\n"
+ "-failingStep\n"
+ "---> STEP FAILED\n"
+ "----> STEP FAILED\n"
+ "---> STEP DONE\n"
+ "TEST DONE\n";
assertThat(consoleStepListener.toString(), is(expectedSteps));
}
Expand All @@ -414,7 +415,7 @@ public void should_be_able_to_record_step_failures_after_a_step_ends() {
+ "---> STEP DONE\n"
+ "-step2\n"
+ "---> STEP DONE\n"
+ "--> STEP FAILED\n"
+ "---> STEP FAILED\n"
+ "TEST DONE\n";
assertThat(consoleStepListener.toString(), is(expectedSteps));
}
Expand Down Expand Up @@ -522,7 +523,8 @@ public void should_skip_steps_after_a_step_failure() {
+ "--step1\n"
+ "----> STEP DONE\n"
+ "--failingStep\n"
+ "----> STEP FAILED\n"
+ "-----> STEP FAILED\n"
+ "----> STEP DONE\n"
+ "--step2\n"
+ "----> STEP IGNORED\n"
+ "---> STEP DONE\n"
Expand Down Expand Up @@ -571,7 +573,8 @@ public void should_skip_nested_steps_after_a_step_failure() {
+ "--step1\n"
+ "----> STEP DONE\n"
+ "--failingStep\n"
+ "----> STEP FAILED\n"
+ "-----> STEP FAILED\n"
+ "----> STEP DONE\n"
+ "--step2\n"
+ "----> STEP IGNORED\n"
+ "---> STEP DONE\n"
Expand All @@ -596,7 +599,8 @@ public void should_not_use_the_browser() {
+ "--step1\n"
+ "----> STEP DONE\n"
+ "--failingStep\n"
+ "----> STEP FAILED\n"
+ "-----> STEP FAILED\n"
+ "----> STEP DONE\n"
+ "--step2\n"
+ "----> STEP IGNORED\n"
+ "---> STEP DONE\n"
Expand Down Expand Up @@ -635,7 +639,8 @@ public void a_step_can_return_a_step_object_if_a_failure_occurs() {
String expectedSteps =
"TEST a_test\n"
+ "-stepThatFailsAndReturnsAStep\n"
+ "---> STEP FAILED\n"
+ "----> STEP FAILED\n"
+ "---> STEP DONE\n"
+ "-stepThatReturnsAStep\n"
+ "---> STEP IGNORED\n"
+ "TEST DONE\n";
Expand Down
6 changes: 0 additions & 6 deletions serenity-cucumber/src/main/java/io/cucumber/core/plugin/ScenarioContextParallel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,6 @@ public void addHighPriorityStepEventBusEvent(String scenarioId, StepEventBusEven

public void addStepEventBusEvent(StepEventBusEvent event) {
if (TestSession.isSessionStarted()) {
if (event instanceof StepFinishedWithResultEvent) {
if (Status.FAILED.equals(((StepFinishedWithResultEvent) event).getResult().getStatus()) && TestSession.currentStepHasFailed()) {
LOGGER.debug("SRP:ignored event " + event + " " + Thread.currentThread() + " because current Step has already failed");
return;
}
}
TestSession.addEvent(event);
event.setStepEventBus(stepEventBus);
} else {
Expand Down
1 change: 1 addition & 0 deletions ...y-cucumber/src/main/java/net/serenitybdd/cucumber/events/StepFinishedWithResultEvent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ private void failed(String stepTitle, Throwable cause, List<ScreenshotAndHtmlSou
getStepEventBus().stepFailed(stepFailure, screenshots, isInDataDrivenTest);
}
}
this.getStepEventBus().stepFinished(this.screenshotList, this.getTimestamp());
}

private void skipped(String stepTitle, Throwable cause) {
Expand Down
4 changes: 2 additions & 2 deletions ...nity-model/src/test/java/net/thucydides/model/requirements/AggregateRequirementsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void should_treat_files_in_a_single_level_directory_structure_as_representing_ca

List<Requirement> requirements = requirementsFrom(pathTo("serenity-js/spec-1-level"));

assertThat(requirements).hasSize(1);
assertThat(requirements).hasSize(2);

Requirement capability = requirements.get(0);
assertThat(capability.getName()).isEqualTo("payments");
Expand All @@ -75,7 +75,7 @@ void should_treat_files_in_a_two_level_directory_structure_as_representing_theme

List<Requirement> requirements = requirementsFrom(pathTo("serenity-js/spec-2-levels"));

assertThat(requirements).hasSize(1);
assertThat(requirements).hasSize(2);

Requirement theme = requirements.get(0);
assertThat(theme.getName()).isEqualTo("ecommerce");
Expand Down

0 comments on commit 992b79d

Please sign in to comment.