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

PrivateMethodTester does not correctly honor type bounds for method arguments #2300

Open
nickmoorman opened this issue Oct 20, 2023 · 1 comment

Comments

@nickmoorman
Copy link

I recently ran into a situation where PrivateMethod/PrivateMethodTester does not seem to correctly honor the type bounds defined for arguments to the method being tested. I put together the code below to illustrate a simplified example of what I'm seeing.

Here are some definitions that mimic the structure of the code used in my application:

// Abstract class and marker traits to set up test conditions
abstract class Thing
trait HasPropertyA
trait HasPropertyB

// Concrete implementations for tests
class ThingWithPropertyA extends Thing with HasPropertyA
class ThingWithPropertiesAAndB extends Thing with HasPropertyA with HasPropertyB

// Object containing methods to test
object TestClass {
  def doSomething[T <: Thing with HasPropertyA with HasPropertyB](
    thing: T
  ): String = {
    s"thing: $thing"
  }

  private def doSomethingPrivately[T <: Thing with HasPropertyA with HasPropertyB](
    thing: T
  ): String = doSomething(thing)
}

This test code (with comments showing the results when run) demonstrates the problem:

import org.scalatest.PrivateMethodTester
import org.scalatest.freespec.AnyFreeSpec
import org.scalatest.matchers.should.Matchers

class PrivateMethodTesterSpec extends AnyFreeSpec with Matchers with PrivateMethodTester {
  val thingA = new ThingWithPropertyA
  val thingAB = new ThingWithPropertiesAAndB

  // PASS
  "doSomething() should work for correctly typed Things" in {
    TestClass.doSomething(thingAB) shouldEqual s"thing: $thingAB"
  }

  // PASS
  "calling doSomething() with incorrectly typed Things should not type check" in {
    "TestClass.doSomething(thingA)" shouldNot typeCheck
  }

  val doSomethingPrivately = PrivateMethod[String](Symbol("doSomethingPrivately"))

  // PASS
  "doSomethingPrivately() should work for correctly typed Things" in {
    TestClass.invokePrivate(doSomethingPrivately(thingAB)) shouldEqual s"thing: $thingAB"
  }

  // FAIL
  "calling doSomethingPrivately() with incorrectly typed Things should not type check" in {
    "TestClass.invokePrivate(doSomethingPrivately(thingA))" shouldNot typeCheck
  }

  // PASS
  "calling doSomethingPrivately() with incorrectly typed Things should not type check but works anyway" in {
    TestClass.invokePrivate(doSomethingPrivately(thingA)) shouldEqual s"thing: $thingA"
  }
}

And here's the raw test output:

[info] PrivateMethodTesterSpec:
[info] - doSomething() should work for correctly typed Things
[info] - calling doSomething() with incorrectly typed Things should not type check
[info] - doSomethingPrivately() should work for correctly typed Things
[info] - calling doSomethingPrivately() with incorrectly typed Things should not type check *** FAILED ***
[info]   Expected a type error, but got none for code: TestClass.invokePrivate(doSomethingPrivately(thingA)) (PrivateMethodTesterSpec.scala:54)
[info] - calling doSomethingPrivately() with incorrectly typed Things should not type check but works anyway
[info] Run completed in 1 second, 214 milliseconds.
[info] Total number of tests run: 5
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 4, failed 1, canceled 0, ignored 0, pending 0
[info] *** 1 TEST FAILED ***

In addition to the type checking not working for the private method, I was very surprised to see that doSomethingPrivately() successfully passes the ThingWithPropertyA to doSomething() and receives a result, despite doSomething() also requiring that the Thing extends both HasPropertyA and HasPropertyB. I made some adjustments to see what happened if doSomething() tried to access a property that only existed in HasPropertyB, and it looks like invokePrivate might be just trying to cast the input arguments as the expected types? This would explain why it works in some cases but not all.

...

trait HasPropertyB {
  val propB: String = "b"
}

...

  def doSomething[T <: Thing with HasPropertyA with HasPropertyB](
    thing: T
  ): String = {
    s"thing: $thing (${thing.propB})"
  }

...

  // NOTE: NO LONGER PASSES!  SEE OUTPUT BELOW
  "calling doSomethingPrivately() with incorrectly typed Things should not type check but works anyway" in {
    TestClass.invokePrivate(doSomethingPrivately(thingA)) shouldEqual s"thing: $thingA (b)"
  }

...
[info] - calling doSomethingPrivately() with incorrectly typed Things should not type check but works anyway *** FAILED ***
[info]   java.lang.ClassCastException: class <mypackage>.ThingWithPropertyA cannot be cast to class <mypackage>.HasPropertyB (<mypackage>.ThingWithPropertyA and <mypackage>.HasPropertyB are in unnamed module of loader 'app')
[info]   at <mypackage>.TestClass$.doSomething(PrivateMethodTesterSpec.scala:24)
[info]   at <mypackage>.TestClass$.doSomethingPrivately(PrivateMethodTesterSpec.scala:29)
[info]   at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[info]   at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
[info]   at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[info]   at java.base/java.lang.reflect.Method.invoke(Method.java:568)
[info]   at org.scalatest.PrivateMethodTester$Invoker.invokePrivate(PrivateMethodTester.scala:260)
[info]   at <mypackage>.PrivateMethodTesterSpec.$anonfun$new$6(PrivateMethodTesterSpec.scala:71)
[info]   at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
[info]   at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
[info]   ...

I'm using ScalaTest 3.2.16, Scala 2.13.10, Java 17.

@cheeseng
Copy link
Contributor

@nickmoorman The current version of PrivateMethodTester of using Any as argument types, I tried to improve it in the following PR:

#2301

Let's see if @bvenners thinks it is a good idea.

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