Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File and line number incorrect when custom Matcher reports failure #254

Closed
chrispcampbell opened this issue Apr 7, 2014 · 5 comments
Closed

Comments

@chrispcampbell
Copy link

We have a few custom Matcher implementations, but unlike the built-in matchers such as beTrue, ours reports a not-terribly-useful file and line number. I haven't dug too deeply into specs2 to confirm this, but I suspect it might be related to the default stack trace filtering that doesn't take our package name into account:

https://github.com/etorreborre/specs2/blob/master/common/src/main/scala/org/specs2/execute/ResultStackTrace.scala#L13

  /** @return the location (file and line number) of the topmost stackTraceElement */
  def location = {
    val filtered = Throwablex.exception(DefaultStackTraceFilter(stackTrace)).getStackTrace
    Throwablex.exception(filtered).location
  }

Perhaps if there was a way for us to include our custom specs2 package name in the set of exclude filters, then it would solve the problem?

The following test code demonstrates the issue:

package coop.plausible.util

import coop.plausible.specs2.matcher.ResultMatchers
import org.specs2.mutable.Specification

class ResultTest extends Specification with ResultMatchers {

  "beTrue" should {
    "match on true" in {
      /*
       * Here's a test using a built-in Matcher that should obviously fail.
       *
       * The line number reported by specs2 is correct and useful:
       *   
       * [info] beTrue should
       * [info] x match on true
       * [error]    the value is false (ResultTest.scala:16)
       */
      false must beTrue
    }
  }

  "beSuccess" should {
    "match on Success" in {
      /*
       * Here's a test using our custom Matcher that should obviously fail.
       *
       * Unfortunately, specs2 reports the line number in our custom Matcher
       * implementation instead of the failing test code:
       *
       * [info] beSuccess should
       * [info] x match on Success
       * [error]    'Failure(42)' is not Success (ResultMatchers.scala:48)
       */
      Failure(42) must beSuccess
    }
  }
}
@etorreborre
Copy link
Owner

You can add additional filtering with the traceFilter argument:

sbt> test-only *ResultTest* -- traceFilter /coop.plausible.*

I don't have time to verify this solution right now, but this is expected to work.

@chrispcampbell
Copy link
Author

Yes, that's how I was originally expecting it would work, but no such luck. So that's how I ended up digging a bit further to try to figure out why the traceFilter argument wasn't taking effect and discovering that line I mentioned earlier in ResultStackTrace where it filters using DefaultStackTraceFilter rather than picking up what the user included in the command-line arguments.

To demonstrate, things work correctly with the following hacked up patch:

--- a/common/src/main/scala/org/specs2/execute/ResultStackTrace.scala
+++ b/common/src/main/scala/org/specs2/execute/ResultStackTrace.scala
@@ -10,7 +10,8 @@ import Throwablex._
 trait ResultStackTrace extends HasStackTrace {
   /** @return the location (file and line number) of the topmost stackTraceElement */
   def location = {
-    val filtered = Throwablex.exception(DefaultStackTraceFilter(stackTrace)).getStackTrace
+    val filter = DefaultStackTraceFilter.excludeAlso("^coop.plausible.specs2")
+    val filtered = Throwablex.exception(filter(stackTrace)).getStackTrace
     Throwablex.exception(filtered).location
   }
   def exception: Throwable

Output:

[info] ResultTest
[info] 
[info] beTrue should
[info] x match on true
[error]    the value is false (ResultTest.scala:19)
[info] 
[info] 
[info] beSuccess should
[info] x match on Success
[error]    'Failure(42)' is not Success (ResultTest.scala:35)

In the interests of coming up with an actual patch, I tried digging further to see if there was a way to use args.traceFilter from that context (instead of the hardcoded DefaultStackTraceFilter), but I kind of got lost in the system and was hoping you'd have a recommendation. (Or let me know if I'm still barking up the wrong tree here.)

@etorreborre
Copy link
Owner

Chris I have a fix for you but can't commit right now. Will do in 4 hours max.

@etorreborre
Copy link
Owner

I forgot to add fixed #254 in ab60f2a.

@chrispcampbell
Copy link
Author

Thanks for the fast turnaround, Eric! I verified that tracefilter now works with your fix in place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants