Cornichon fails silently when exception is raised in hooks

[Isammoc]

I was fighting with my scenario when it occurs to me, that my afterFeature hook may raise an error (in case of errors in the scenario, some resources are not clean for now).

As Cornichon is a test framework, I expected that when an exception happened, I know at least which exception happened and in which block, but I only had a basic message that my feature is unsuccessful.

Considering this code:

import com.github.agourlay.cornichon.CornichonFeature
import com.github.agourlay.cornichon.core.FeatureDef
import com.github.agourlay.cornichon.steps.regular.DebugStep

class ExceptionFeature extends CornichonFeature {
  override def feature: FeatureDef = Feature("Sample") {
    Scenario("Dummy") {
      When I DebugStep("do nothing", _ => Right("always successful"))
    }
  }

  afterFeature {
    throw new Exception("I failed")
    ()
  }
}

The result was:

...snip compilation...
[info] Done compiling.
Starting scenario 'Dummy'
[error] Error: Total 2, Failed 0, Errors 1, Passed 1
[error] Error during tests:
[error] 	ExceptionFeature
[error] (Test / testOnly) sbt.TestsFailedException: Tests unsuccessful
[error] Total time: 9 s, completed Jul 7, 2020 3:24:28 PM

Process finished with exit code 1

Expectation

The exception is caught and we can see the report with an obvious error case in afterFeature block about raising an java.lang.Exception exception with the message "I failed". Kudos if the stack trace is meaningful.

Workaround

Once I found that the issue was with exceptions, I overridden beforeFeature and afterFeature as following:

import scala.util.Try

import com.github.agourlay.cornichon.CornichonFeature
import org.log4s._

trait CustomCornichonFeature extends CornichonFeature {

  override def beforeFeature(before: => Unit): Unit = {
    super.beforeFeature {
      logButRethrow(before, "Exception during beforeFeature")
    }
  }

  override def afterFeature(after: => Unit): Unit = super.afterFeature {
    logButRethrow(after, "Exception during afterFeature")
  }

  private def logButRethrow(before: => Unit, msg: String): Unit = {
    Try(before).fold(t => {
      getLogger.error(t)(msg)
      throw t
    }, identity)
  }
}

Notes:

• I don't like to rethrow the exception. But I think it is necessary for now to be able to handle the failure case (and not beginning the real feature / the scenarios if setUp failed in beforeFeature block)
• I am not sure that log4s is a dependency of Cornichon or my own project.
• In real implementation, I don't know if we have to manage each block in isolation or if the first failed, we do not let the others to execute.

[agourlay]

Thank you very much for the detailed analysis & description 👍

I am sorry you lost time finding the issue as one of the main value of Cornichon is to have great error reporting in all situations.

This definitely definitely like a bug, it is caused by the fact that the beforeFeature and afterFeature hooks more ad-hoc workarounds than real part of the execution model.

This is the moment where I bring up the oldest issue tracking potential improvements there #71

For the time being I will make sure to handle this case properly in the internals.

I don't like to rethrow the exception. But I think it is necessary for now to be able to handle the failure case (and not beginning the real feature / the scenarios if setUp failed in beforeFeature block)

I propose to catch the exception and to add information to it before letting it bubble up.

I am not sure that log4s is a dependency of Cornichon or my own project.

This dependency is pulled-in by http4s, it is not used in the library.

In real implementation, I don't know if we have to manage each block in isolation or if the first failed, we do not let the others to execute.

This kind of problem is solved by ResourceScenarioStep to make sure we actually run the clearning phase.
In that case, I think we have to do it manually and try our best to run the afterFeature as soon as a single error is thrown within beforeFeature.

[agourlay]

I propose this to harden it #384

[agourlay]

The fix will be part of 0.19.3 next week.

[Isammoc]

Thank you to have took this into account. And for the fix.

[agourlay]

I am really happy to have harden this part of the lib 🔨

Please give it a try to check if it handles your case and has a clear error reporting.