Skip to content

Commit

Permalink
Improved exception reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
wakaleo committed Jan 10, 2016
1 parent 6dd8c5e commit b5a6c97
Show file tree
Hide file tree
Showing 16 changed files with 139 additions and 55 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package net.serenitybdd.screenplay.questions.page;

import net.serenitybdd.screenplay.Actor;
import net.serenitybdd.screenplay.Question;
import net.serenitybdd.screenplay.abilities.BrowseTheWeb;

public class AlertTextQuestion implements Question<String> {
@Override
public String answeredBy(Actor actor) {
return BrowseTheWeb.as(actor).getAlert().getText();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package net.serenitybdd.screenplay.questions.page;

import net.serenitybdd.screenplay.Actor;
import net.serenitybdd.screenplay.Question;
import net.serenitybdd.screenplay.abilities.BrowseTheWeb;

public class PageTitleQuestion implements Question<String> {
@Override
public String answeredBy(Actor actor) {
return BrowseTheWeb.as(actor).getTitle();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package net.serenitybdd.screenplay.questions.page;

public class TheWebPage {
public static PageTitleQuestion title() {
return new PageTitleQuestion();
}

public static AlertTextQuestion alertText() {
return new AlertTextQuestion();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

public class UnrecognisedException extends Throwable {

public UnrecognisedException(String message) {
super(message);
public UnrecognisedException() {
super();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,12 @@ private Optional<? extends Throwable> restoreExceptionFrom(String testFailureCla
Class failureClass = Class.forName(testFailureClassname);
Throwable exception = buildThrowable(testFailureMessage, failureClass);
if (exception == null) {
exception = new UnrecognisedException(failureClass.getName() + ": " + testFailureMessage);
exception = new UnrecognisedException();
}
exception.setStackTrace(this.getStackTrace());
return Optional.fromNullable(exception);
} catch (Exception e) {
Throwable exception = new UnrecognisedException(testFailureClassname + ": " + testFailureMessage);
Throwable exception = new UnrecognisedException();
exception.setStackTrace(this.getStackTrace());
return Optional.fromNullable(exception);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package net.thucydides.core.model.stacktrace;

import net.serenitybdd.core.exceptions.SerenityWebDriverException;
import net.thucydides.core.guice.Injectors;
import net.thucydides.core.model.failures.FailureAnalysis;
import net.thucydides.core.util.EnvironmentVariables;
import net.thucydides.core.webdriver.WebdriverAssertionError;

/**
Expand All @@ -9,9 +12,13 @@
public class RootCauseAnalyzer {

private final Throwable thrownException;
private final FailureAnalysis failureAnalysis;
private final EnvironmentVariables environmentVariables;

public RootCauseAnalyzer(Throwable thrownException) {
this.thrownException = thrownException;
this.environmentVariables = Injectors.getInjector().getInstance(EnvironmentVariables.class);
failureAnalysis = new FailureAnalysis(environmentVariables);
}

public FailureCause getRootCause() {
Expand All @@ -22,9 +29,19 @@ public FailureCause getRootCause() {
}

private Throwable originalExceptionFrom(Throwable thrownException) {

if (!(thrownException instanceof WebdriverAssertionError) && ((thrownException instanceof SerenityWebDriverException) || (thrownException instanceof AssertionError))){
return thrownException;
}
if (failureAnalysis.reportAsCompromised(thrownException.getClass())) {
return thrownException;
}
if (failureAnalysis.reportAsFailure(thrownException.getClass())) {
return thrownException;
}
if (failureAnalysis.reportAsPending(thrownException.getClass())) {
return thrownException;
}
return (thrownException.getCause() != null) ? thrownException.getCause() : thrownException;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,18 @@ private void checkTestResultsIn(TestOutcomes testOutcomes) {
switch (testOutcomes.getResult()) {
case ERROR: throw new TestOutcomesError(testOutcomeSummary(testOutcomes));
case FAILURE: throw new TestOutcomesFailures(testOutcomeSummary(testOutcomes));
case COMPROMISED: throw new TestOutcomesCompromised(testOutcomeSummary(testOutcomes));
}
}

private String testOutcomeSummary(TestOutcomes testOutcomes) {
int errors = testOutcomes.count(TestType.ANY).withResult(TestResult.ERROR);
int failures = testOutcomes.count(TestType.ANY).withResult(TestResult.FAILURE);
int compromised = testOutcomes.count(TestType.ANY).withResult(TestResult.COMPROMISED);
String errorText = (errors > 0) ? "ERROR COUNT: " + errors : "";
String failureText = (failures > 0) ? "FAILURE COUNT: " + failures : "";
return "THUCYDIDES TEST FAILURES: " + errorText + " " + failureText;
String compromisedText = (compromised > 0) ? "COMPROMISED COUNT: " + failures : "";
return "THUCYDIDES TEST FAILURES: " + errorText + " " + failureText + " " + compromisedText;
}

private void handleMissingTestResults() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package net.thucydides.core.reports;

/**
* Created by john on 10/01/2016.
*/
public class TestOutcomesCompromised extends AssertionError {
public TestOutcomesCompromised(String message) {
super(message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ private void recordFailureDetails(final StepFailure failure) {
}

private void addTagFor(TestOutcome testOutcome) {
testOutcome.addTag(TestTag.withName(testOutcome.getTestFailureCause().getSimpleErrorType()).andType("error"));
testOutcome.addTag(TestTag.withName(testOutcome.getTestFailureCause().getSimpleErrorType()).andType("error type"));
}

private boolean shouldTagErrors() {
Expand Down Expand Up @@ -762,7 +762,9 @@ public WebDriver getDriver() {

public boolean aStepHasFailed() {
return ((!getTestOutcomes().isEmpty()) &&
(getCurrentTestOutcome().getResult() == TestResult.FAILURE || getCurrentTestOutcome().getResult() == TestResult.ERROR));
(getCurrentTestOutcome().getResult() == TestResult.FAILURE
|| getCurrentTestOutcome().getResult() == TestResult.ERROR
|| getCurrentTestOutcome().getResult() == TestResult.COMPROMISED));
}

public FailureCause getTestFailureCause() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package net.thucydides.core.model

import net.serenitybdd.core.exceptions.CausesCompromisedTestFailure
import net.thucydides.core.model.stacktrace.RootCauseAnalyzer
import net.thucydides.core.model.stacktrace.StackTraceSanitizer
import net.thucydides.core.util.MockEnvironmentVariables
import org.openqa.selenium.ElementNotVisibleException
import sample.steps.FailingStep
import spock.lang.Specification
import spock.lang.Unroll
Expand All @@ -20,6 +22,34 @@ class WhenDeterminingTheRootCauseOfAnError extends Specification {
rootCause.stackTrace.size() == 1
}

def "Should report WebdriverAssertionError exceptions directly "() {
given:
def exception = new ElementNotVisibleException("Oh crap")
when:
def rootCause = new RootCauseAnalyzer(exception).getRootCause()
then:
rootCause.errorType == "org.openqa.selenium.ElementNotVisibleException"
}

public static class AppCompromised extends Error implements CausesCompromisedTestFailure {
AppCompromised(String message) {
super(message)
}

AppCompromised(String message, Throwable cause) {
super(message, cause)
}
}

def "Should report Compromised exceptions directly "() {
given:
def exception = new AppCompromised("Oh crap", new AssertionError("Oh bugger"))
when:
def rootCause = new RootCauseAnalyzer(exception).getRootCause()
then:
rootCause.errorType.contains("AppCompromised")
}

def "Should record full stack trace if configured"() {
given:
def exception = new FailingStep().failsWithMessage("Oh crap")
Expand Down
6 changes: 6 additions & 0 deletions serenity-core/src/test/java/sample/steps/FailingStep.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package sample.steps;

import org.openqa.selenium.ElementNotVisibleException;

/**
* Created by john on 4/07/2014.
*/
Expand All @@ -8,4 +10,8 @@ public class FailingStep {
public Exception failsWithMessage(String message) {
return new IllegalArgumentException(message);
}

public Exception failsWith(ElementNotVisibleException cause) {
return cause;
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package net.serenitybdd.screenplay;

import net.thucydides.core.steps.StepEventBus;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public abstract class BaseConsequence<T> implements Consequence<T> {

private Class<? extends Error> complaintType;
private String complaintDetails;

private final Logger LOGGER = LoggerFactory.getLogger(this.getClass());

protected Error errorFrom(Throwable actualError) {
if (actualError instanceof Error) {
return (Error) actualError;
Expand All @@ -16,6 +20,7 @@ protected Error errorFrom(Throwable actualError) {

protected void throwComplaintTypeErrorIfSpecified(Error actualError) {
if (complaintType != null) {
LOGGER.error("Could not resolve question", actualError);
throw Complaint.from(complaintType, complaintDetails, actualError);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package net.serenitybdd.screenplay;

import static com.google.common.base.Optional.fromNullable;

public class Complaint {

public static final String NO_VALID_CONSTRUCTOR = "%s should have a constructor with the signature MyException(String message) and MyException(String message, Throwable cause)";
Expand All @@ -14,7 +12,7 @@ public static Error from(Class<? extends Error> complaintType,
return from(complaintType, actualError);
}

complaintDetails = fromNullable(complaintDetails).or(actualError.getMessage());
complaintDetails = errorMessageFrom(complaintDetails, actualError);

try {
return complaintType.getConstructor(String.class, Throwable.class).newInstance(complaintDetails, actualError);
Expand All @@ -23,6 +21,16 @@ public static Error from(Class<? extends Error> complaintType,
}
}

private static String errorMessageFrom(String complaintDetails, Error actualError) {
if (complaintDetails == null) {
return actualError.getMessage();
}
if (actualError.getMessage() == null) {
return complaintDetails;
}
return complaintDetails + " - " + actualError.getMessage();
}

public static Error from(Class<? extends Error> complaintType, Throwable actualError) {
try {
return complaintType.getConstructor(String.class, Throwable.class).newInstance(actualError);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package net.serenitybdd.screenplay;

public class ExternalValueQuestion {

public static <T> Question<T> valueOf(final T value) {
return new Question<T>() {
@Override
public T answeredBy(Actor actor) {
return value;
}
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,4 @@ public static <T> void then(T actual, Matcher<? super T> matcher) {
public static <T> Consequence<T> seeThat(Question<? extends T> actual, Matcher<T> expected) {
return new QuestionConsequence(actual, expected);
}

}

This file was deleted.

0 comments on commit b5a6c97

Please sign in to comment.