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

Broken spying on functions #1033

Open
GFriedrich opened this issue Jan 28, 2023 · 7 comments
Open

Broken spying on functions #1033

GFriedrich opened this issue Jan 28, 2023 · 7 comments

Comments

@GFriedrich
Copy link

GFriedrich commented Jan 28, 2023

Expected Behavior

Calling a line like val test: (Int) -> Unit = spyk({}) shouldn't fail.

Current Behavior

Running this line fails with:

This class is an internal synthetic class generated by the Kotlin compiler, such as an anonymous class for a lambda, a SAM wrapper, a callable reference, etc. It's not a Kotlin class or interface, so the reflection library has no idea what declarations it has.

Failure Information (for bugs)

This got likely broken because of the ProxyMaker deduplication of #1024

Steps to Reproduce

  1. Add a line like val test: (Int) -> Unit = spyk({}) to a unit test
  2. Run the test

Context

  • MockK version: 1.13.4
  • Kotlin version: 1.6
  • JDK version: 17
  • Type of test: unit test
@SimonMarquis
Copy link
Contributor

I can confirm this worked on the previous version 1.13.3.
The current workaround for us has been to do the following:

val lambda = spyk<() -> Unit>()
// or
val lambda: () -> Unit = spyk()

@sampengilly
Copy link

sampengilly commented Jan 31, 2023

For non-trivial spy'd lambdas such as ones that need to return a value, the following seems to be necessary:

val lambda: (Int, String) -> Boolean = spyk {
    every { this@spyk.invoke(any(), any()) } answers { ... }
}

EDIT: Or moving the every out of the spyk {} block

@JohannesTeichert
Copy link

JohannesTeichert commented Feb 1, 2023

For non-trivial spy'd lambdas such as ones that need to return a value, the following seems to be necessary:

val lambda: (Int, String) -> Boolean = spyk {
    every { this@spyk.invoke(any(), any()) } answers { ... }
}

EDIT: Or moving the every out of the spyk {} block

Thanks for this answer, helped me find a solution for my very similar problem.

I had to change me spyk instantiation from this:

val userListObserver: Observer<List<User>> = spyk(Observer { })

to this:

val userListObserver: Observer<List<User>> = spyk { Observer<List<User>>{  } }

what I still get which is new since the update is this slf4j logger provider warning:

SLF4J: No SLF4J providers were found.
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See https://www.slf4j.org/codes.html#noProviders for further details.

@krissrex
Copy link

krissrex commented Feb 3, 2023

what I still get which is new since the update is this slf4j logger provider warning:

SLF4J: No SLF4J providers were found.
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See https://www.slf4j.org/codes.html#noProviders for further details.

SLF4J version 2.0.5 is a dependency of mockk.

This specific issue happens when your slf4j version is updated, but your logback version is old.
They had a breaking update recently.
Try:

<slf4j.version>2.0.6</slf4j.version>
<logback.version>1.4.5</logback.version>

@krissrex
Copy link

krissrex commented Feb 3, 2023

I have the same problem when spying on fun interface implementations:

Fails:

spyk(
  MyFunInterface { 1 + 2 }
)

Works:

spyk(
  object : MyFunInterface {
    override fun doThing(): Int = 1 + 2
  }
)

@heli-os
Copy link

heli-os commented Feb 5, 2023

I'm experiencing the same problem in 1.13.4. I downgraded to 1.13.3, and it is being used temporarily.

ex)

interface Logger {

    fun interface Factory {
        fun getLogger(name: String): Logger

        fun getLogger(clazz: Class<*>): Logger = getLogger(clazz.name)
    }
}

// Error Occurred
val sut = spyk(Logger.Factory { NoOpLogger })
sut.getLogger(LoggerTest::class.java)
verify(exactly = 1) {
    sut.getLogger(any<String>())
}

OresteSalerno-TomTom added a commit to tomtom-international/kotlin-tools that referenced this issue Jul 6, 2023
Improve performance of `Tracer.invoke()`:
- ~70% improvement if there are no trace event consumers
- ~55% improvement if there are trace event consumers

Changes:
* Introduce caching of trace event method annotations in the `Tracer.invoke()` function.
* Skip creation of a `TraceEvent` object if there are no event consumers. This saves us from performing an expensive
  `LocalDateTime.now()` call.
* Skip retrieval of trace event method annotations for simple log function invocations, if there are no event consumers.
* Remove string concatenations in `createLogMessage()` functions. Only use `StringBuilder` functions.
* Change `toStringRegistry` to use `item::javaClass.name` as key, and use `item::class.toString()` only as fallback.
* Skip reading `item.javaClass.getMethod("toString").declaringClass` in `convertToStringUsingRegistry()`: the
  `item.toString()` function of `Any` will anyway print the name of the class.
* Fix execution of unit tests requiring mockk-jvm (`extensions` and `traceevents` modules): no unit test for these
  modules was being run anymore, after the 1.8.2 update.
* Fix the `IfExtensionsTest` unit tests: these tests were broken by mockk 1.13.4 (due to
  mockk/mockk#1033).
RijnBuve-TomTom pushed a commit to tomtom-international/kotlin-tools that referenced this issue Jul 6, 2023
* Improve performance of Tracer.invoke() function

Improve performance of `Tracer.invoke()`:
- ~70% improvement if there are no trace event consumers
- ~55% improvement if there are trace event consumers

Changes:
* Introduce caching of trace event method annotations in the `Tracer.invoke()` function.
* Skip creation of a `TraceEvent` object if there are no event consumers. This saves us from performing an expensive
  `LocalDateTime.now()` call.
* Skip retrieval of trace event method annotations for simple log function invocations, if there are no event consumers.
* Remove string concatenations in `createLogMessage()` functions. Only use `StringBuilder` functions.
* Change `toStringRegistry` to use `item::javaClass.name` as key, and use `item::class.toString()` only as fallback.
* Skip reading `item.javaClass.getMethod("toString").declaringClass` in `convertToStringUsingRegistry()`: the
  `item.toString()` function of `Any` will anyway print the name of the class.
* Fix execution of unit tests requiring mockk-jvm (`extensions` and `traceevents` modules): no unit test for these
  modules was being run anymore, after the 1.8.2 update.
* Fix the `IfExtensionsTest` unit tests: these tests were broken by mockk 1.13.4 (due to
  mockk/mockk#1033).

* Address review comments
@carterhudson
Copy link

carterhudson commented Sep 7, 2023

This is still a problem in 1.13.7.
I would use 1.13.3, but it has this issue.
:(

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

7 participants