michaelbull / kotlin-result

A multiplatform Result monad for modelling success or failure operations.
ISC License
1.06k stars 64 forks source link

Issues with @JvmStatic code #102

Closed Takiguchi72 closed 7 months ago

Takiguchi72 commented 8 months ago

Hello, I'm using the "Result library" on a spring boot kotlin project.

I faced to problems while I used some "Result" objects in a parameterized test with a method source annotated with @JvmStatic:

My environment

Tool type Tool Version
Result library - 2.0.0
OS MacOS (aarch64) 14.4
JDK Amazon corretto 17.0.9
Kotlin SDK - 1.9.23
JUnit Jupiter 5

Class cast exception problem

Here is an example of my code:

class MyServiceTest {
  private myService: MyService

  @BeforeEach
  fun setUp() {
    myService = MyService(...)
  }

  @Nested
  inner class MyFunction {
    @ParameterizedTest
    @MethodSource("package.of.my.project.MyServiceTest#argumentsOfMyTest")
    fun `my test`(
      firstResult: Result<List<Customer>, CustomerError>,
      secondResult: Result<Customer, CustomerError>
    ) = runTest {
      // given
      ...

      // when
      val result = myService.myFunction()

      // then
      result shouldBe ...

      firstResult.onSuccess {
        coVerify (exactly = 1) {
            // another service is called with some parameters
        }
      }
    }
  }

  private companion object {
    @JvmStatic
    fun argumentsOfMyTest(): Stream<Arguments> = Stream.of(
        Arguments.of(
            Ok(listOf(...)),
            Err(CustomerError("error details"))
        )
        // ...
    )
  }
}

Note: onSuccess method calls the value getter inside the Result class.

The error at runtime:

java.lang.ClassCastException: class com.github.michaelbull.result.Result cannot be cast to class java.util.List (com.github.michaelbull.result.Result is in unnamed module of loader 'app'; java.util.List is in module java.base of loader 'bootstrap')

Note: I tried to use a non "jvmstatic" method, and I don't reproduce the error.

Example without the "jvmstatic" code :

class MyServiceTest {
  private myService: MyService

  @BeforeEach
  fun setUp() {
    myService = MyService(...)
  }

  @TestInstance(PER_CLASS) // <-- "static import" of enum value (TestInstance.Lifecycle)
  @Nested
  inner class MyFunction {
    @ParameterizedTest
    @MethodSource("package.of.my.project.MyServiceTest#argumentsOfMyTest")
    fun `my test`(
      firstResult: Result<List<Customer>, CustomerError>,
      secondResult: Result<Customer, CustomerError>
    ) = runTest {
      // given
      ...

      // when
      val result = myService.myFunction()

      // then
      result shouldBe ...

      firstResult.onSuccess {
        coVerify (exactly = 1) {
            // another service is called with some parameters
        }
      }
    }

    private fun argumentsOfMyTest(): Stream<Arguments> = Stream.of(
        Arguments.of(
            Ok(listOf(...)),
            Err(CustomerError("error details"))
        )
        // ...
    )
  }
}

Note: with the @TestInstance(PER_CLASS), I faced to other issues so I don't want use this way.

The error Result that has "isOk==true" problem

Always with this context, I tried to replace the onSuccess by a if bloc that tests isOk attribute.

My unit test fails and when I glance at result state while debugging, I saw its isOk value returns true when my Result is an error.

Code:

class MyServiceTest {
  private myService: MyService

  @BeforeEach
  fun setUp() {
    myService = MyService(...)
  }

  @Nested
  inner class MyFunction {
    @ParameterizedTest
    @MethodSource("package.of.my.project.MyServiceTest#argumentsOfMyTest")
    fun `my test`(
      firstResult: Result<List<Customer>, CustomerError>,
      secondResult: Result<Customer, CustomerError>
    ) = runTest {
      // given
      ...

      // when
      val result = myService.myFunction()

      // then
      result shouldBe ...

      if (firstResult.isOk) {
        coVerify (exactly = 1) {
            // another service is called with some parameters
        }
      }
    }
  }

  private companion object {
    @JvmStatic
    fun argumentsOfMyTest(): Stream<Arguments> = Stream.of(
        //...
        Arguments.of(
            Err(CustomerError("error details")), // <-- Result is an Err, so "isOk" should return "false"
            Ok(...)
        )
    )
  }
}

While debugging, on the line that contains if (firstResult.isOk) {, if result is an Err(...), firstResult.isOk gives true instead of false.

My analyze

After looking the source code of class Result, I think the problem comes from the inlineValue attribute that his type of Any?.

The class cast exception occurs at this moment:

@JvmInline
public value class Result<out V, out E> internal constructor(
    private val inlineValue: Any?,
) {
    @Suppress("UNCHECKED_CAST") // <-- Why? Can a "reified" solve this warning?
    public val value: V
        get() = inlineValue as V // <-- Here is thrown the "Class cast exception"

    // ...
}

At compilation phase, I think some information is lost when I use a Result<List<XXX>, ...> but it only occurs when I use a method annotated @JvmStatic. I don't face this issue with code that is not annonated @JvmStatic.

Maybe a reified value could solve the problem...?

Thanks in advance for your reply!

michaelbull commented 7 months ago

Could you provide a simple case to reproduce the behaviours you are finding erroneous? It looks like you have several libraries going on here that could each be responsible.

Internally a call to isOk is checking if the inlined value is an instanceof Failure, which is what the Err function wraps your value in. The only time that should return false is if you haven't called Err somewhere.

Takiguchi72 commented 7 months ago

Hello, thanks for your reply.

I built a sample gradle project that contains some code to reproduce my bug with the kotlin-result library.

You'll find it there: https://github.com/Takiguchi72/kotlin-result-issue-102-example

You only have to run this command:

./gradlew build

Or run the parameterized unit test in class OneServiceTest if you open the project in an IDE like IntelliJ.

You'll find a link to a report with the error stack trace in the terminal: image

image

Thanks in advance

michaelbull commented 7 months ago

This seems to be a bug with JUnit's parameterised tests, almost certainly related to https://github.com/junit-team/junit5/issues/2703.

You can confirm this yourself with the following experiment:

    @Nested
    inner class DoSomething {
        @ParameterizedTest
        @MethodSource("com.example.demo.OneServiceTest#argumentsOfMyTest")
        fun `test name`(firstResult: Result<Customer, CustomerError>) {
            oneService.doSomething()
        }
    }

    private companion object {
        @JvmStatic
        fun argumentsOfMyTest() = Stream.of(
            Arguments.of(
                Err(CustomerError("")),
            )
        )
    }

If you look at the test above in the debugger, JUnit is actually wrapping your Err(CustomerError("")) in another Result, that is itself Ok:

image

See how the watch on firstResult.toString() is telling us that it's actually an Ok wrapping an Err? This means the outer Result is Ok, but the value that the outer Result holds is another Result that is an Err. It seems like JUnit is magically wrapping the Err(CustomerError("")) with another Result, which is itself therefore Ok.

Interestingly, you can stop JUnit doing this magic behaviour by typing your argument as an Any, notice in the screenshot below that it hasn't magically wrapped it with another Result:

image

Therefore the behaviour you've identified that made it look like an Err was returning true for isOk is not accurate, JUnit has magically wrapped your Err in another Ok, which itself isOk as we would expect. I don't know how you would opt out of this behaviour in any way other than making your arguments a typeof Any to avoid this weird casting behaviour. I suspect this is something you'll have to ask the JUnit team to opt out of.

Takiguchi72 commented 7 months ago

Thanks for your analyze, I'll dig up the Junit thread because source problem is not solved yet. I close this issue.