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

Passing position explicitly to test in Scala 3 #2262

Open
liontiger23 opened this issue Aug 9, 2023 · 5 comments
Open

Passing position explicitly to test in Scala 3 #2262

liontiger23 opened this issue Aug 9, 2023 · 5 comments

Comments

@liontiger23
Copy link

The Scala 3 version of test no longer has implicit source.Position parameter. See for example

* @throws NullArgumentException if <code>testName</code> or any passed test tag is <code>null</code>
*/
// SKIP-DOTTY-START
protected def test(testName: String, testTags: Tag*)(testFun: => Any /* Assertion */)(implicit pos: source.Position): Unit = {
testImpl(testName, testTags: _*)(testFun, pos)
}
// SKIP-DOTTY-END
//DOTTY-ONLY inline def test(testName: String, testTags: Tag*)(testFun: => Any /* Assertion */): Unit = {
//DOTTY-ONLY ${ source.Position.withPosition[Unit]('{(pos: source.Position) => testImpl(testName, testTags: _*)(testFun, pos) }) }
//DOTTY-ONLY }

In our team, we have used this feature to grab position of each test case data and pass it to a test later, so that when it is run in IDEA, double-clicking failed test case will lead to the test case data, from which given test was generated.
Here is an example:

/** Attaches position to test arguments (for bulk tests). */
def tp[T](x: T)(implicit pos: source.Position): (T, source.Position) = (x, pos)

for (((x, y, res), pos) <- Seq(
  tp(2, 2, 4),
  tp(-10, 0, -10),
  ...
)) {
  test(s"add of $x and $y") {
    (x + y) shouldBe res
  }
}

Having separate test for each test case is handy, since they will be run separately and in the resulting log we can see which test cases failed. So having each separate test associated with position of corresponding test case is very convenient for such situation.

If this change in API of test in Scala 3 version is not intentional, maybe it can be restored in the later versions?

Or maybe there is a better way to achieve a similar workflow without abusing implicit position. Any suggestions are welcome.

@cheeseng
Copy link
Contributor

@liontiger23 Hmm, sorry for breaking your use case in Scala 3 build, that was indeed a very creative way of using the implicit position, we did change that intentionally in Scala 3 when we tried to support Scala 3 in its pre-release versions, we were trying to do it the 'Scala 3' way, let's try to discuss with @bvenners and see what he thinks, I'll try to think if there's a workaround for your case first.

@cheeseng
Copy link
Contributor

cheeseng commented Aug 10, 2023

@liontiger23 One workaround may be you can pass the position through the assertion/matchers functions, here's a quick example:

import org.scalactic._
import org.scalatest.funsuite.AnyFunSuite
import org.scalatest.matchers.should.Matchers

class TestSpec extends AnyFunSuite with Matchers {

  /** Attaches position to test arguments (for bulk tests). */
  def tp[T](x: T)(implicit pos: source.Position): (T, source.Position) = (x, pos)

  for (((x, y, res), pos) <- Seq(
    tp(2, 2, 4),
    tp(-10, 1, -10)  // Failure will be pointing to here.
  )) {
    test(s"add of $x and $y") {
        given source.Position = pos
        (x + y) shouldBe res 
    }
  }

}

Hope this helps.

Cheers.

@liontiger23
Copy link
Author

liontiger23 commented Aug 10, 2023

Thanks for the suggestion!

The error does point to a correct location, but unfortunately IDEA does not resolve this link correctly.
Without workaround:

-9 was not equal to -10
ScalaTestFailureLocation: TestSpec at (TestSpec.scala:15)
Expected :-10
Actual   :-9

With workaround:

-9 was not equal to -10
ScalaTestFailureLocation: org.scalatest.matchers.MatchersHelper$ at (TestSpec.scala:12)
Expected :-10
Actual   :-9

Probably the org.scalatest.matchers.MatchersHelper$ confuses IDEA UI and the link TestSpec.scala:12 points to org.scalatest.matchers.MatchersHelper.scala:12.

I have tried a similar workaround abusing withFixture:

import org.scalactic.*
import org.scalatest.Failed
import org.scalatest.funsuite.AnyFunSuite
import org.scalatest.matchers.should.Matchers

class TestSpec extends AnyFunSuite with Matchers {

  /** Attaches position to test arguments (for bulk tests). */
  def tp[T](x: T)(implicit pos: source.Position): (T, source.Position) = (x, pos)

  for (((x, y, res), pos) <- Seq(
    tp(2, 2, 4),
    tp(-10, 1, -10)  // Failure will be pointing to here.
  )) {
    test(s"add of $x and $y") {
      thisPos = pos
      (x + y) shouldBe res
    }
  }

  var thisPos = source.Position.here

  override def withFixture(test: NoArgTest) = {
    super.withFixture(test) match {
      case Failed(e) => Failed.here(e)(thisPos)
      case x => x
    }
  }

}

For some reason this one is resolved correctly by IDEA, so org.scalatest.Failed$ does not confuse IDE here:

-9 was not equal to -10
ScalaTestFailureLocation: org.scalatest.Failed$ at (TestSpec.scala:13)
Expected :-10
Actual   :-9

Although these workarounds do help in some capacity, we lose here the actual line, where the error occurred, because workaround effectively replaces that position with the test case one.

If there were multiple assertions/matchers in test we would not be able to distinguish them without something like withClue.

@cheeseng
Copy link
Contributor

One option is we change to pass in source.Position like this:

5db8730

That will be consistent with other like assertions and matchers also, let's see if @bvenners agree with that.

@raquo
Copy link

raquo commented Dec 3, 2023

Hello, I am having a similar problem with other ScalaTest methods / keywords.

The suggested given source.Position = pos workaround does not do anything in my case.

For a trivial example, I'm trying to add an assertEquals method like this in Scala 3:

import org.scalactic.Prettifier
import org.scalatest.funspec.AnyFunSpec
import org.scalatest.{Assertion, Assertions}
import org.scalactic.source.Position
import org.scalactic.CanEqual

class UnitSpec extends AnyFunSpec with MyMatchers {
 // my tests extend this class
}

trait MyMatchers { this: Assertions =>

  def assertEquals[L, R](
    actual: L,
    expected: R
  )(
    using prettifier: Prettifier,
    pos: Position,
    caneq: CanEqual[L, R]
  ): Assertion = {
    assertResult(expected = expected)(actual = actual)
  }

}

The provided pos is not getting picked up by assertResult: the error it prints points to the line assertResult(expected = expected)(actual = actual), not to the line captured upstream in pos.

Adding given Position = pos before assertResult does not make a difference.

This worked in Scala 2, but it seems impossible to achieve in Scala 3. The new inline def assertResult method calls into a private method, so I can't bypass it by calling the implementation directly.

There are a bunch of other methods in Assertions.scala that use the same source.Position.withPosition pattern, and all the ones I tried are also affected.

Is there really no way to pass my source.Position to assertResult or any of these other methods?

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

3 participants