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

Experimental SKIP feature unnecessary context evaluation #33

Open
mkobit opened this issue May 22, 2019 · 2 comments
Open

Experimental SKIP feature unnecessary context evaluation #33

mkobit opened this issue May 22, 2019 · 2 comments

Comments

@mkobit
Copy link

mkobit commented May 22, 2019

Version: 1.7.0

I was trying out the dev.minutest.experimental.SKIP feature and noticed that it looks like deriveFixture/modifyFixture/probably other pieces are executed for a nested context that is skipped. I was using Mockito mocks in a fixture and was modifying the behavior in each nested context. At the most nested context, I had a handful of tests that I wanted to skip because they were not implemented yet, so I thought I would try out SKIP. I saw test failures due to using strict mocks, so the expectations set on the mocks were throwing org.mockito.exceptions.misusing.UnnecessaryStubbingException after the test because it looks like the behavior settings were called even though the nested context was skipped.

My expectation is that the whole buildup would not be executed for skipped contexts.

Here is an example that I think demonstrates my expectations better than my explanation:

import dev.minutest.experimental.SKIP
import dev.minutest.experimental.minus
import dev.minutest.junit.JUnit5Minutests
import dev.minutest.rootContext

internal class Mk : JUnit5Minutests {
    fun tests() = rootContext<Int> {
        fixture { 0 }
        context("l1") {
            deriveFixture {
                println("deriveFixture: ${it.name}")
                this + 1
            }
            context("l2") {
                test("l2 test") {
                    println(it.name)
                }
            }

            SKIP - context("l2 skipped") {
                test("l2 skipped test") {
                    println(it.name)
                }
            }
        }
    }
}

Output:

deriveFixture: l2 test
l2 test
deriveFixture: l2 skipped

Test ignored.

dev.minutest.experimental.MinutestSkippedException
	at dev.minutest.experimental.InexcludingKt$skipped$1.invoke(inexcluding.kt:47)
	at dev.minutest.experimental.InexcludingKt$skipped$1.invoke(inexcluding.kt)
	at dev.minutest.Test.invoke(Node.kt)
	at dev.minutest.Test.invoke(Node.kt:37)
	at dev.minutest.internal.PreparedContext$runTest$1.invoke(PreparedContext.kt:21)
	at dev.minutest.internal.PreparedContextKt.tryMap(PreparedContext.kt:61)
	at dev.minutest.internal.PreparedContextKt.access$tryMap(PreparedContext.kt:1)
	at dev.minutest.internal.PreparedContext.runTest(PreparedContext.kt:21)
	at dev.minutest.internal.TestExecutor$andThen$1$runTest$testletForParent$1.invoke(TestExecutor.kt:25)
	at dev.minutest.internal.TestExecutor$andThen$1$runTest$testletForParent$1.invoke(TestExecutor.kt:17)
	at dev.minutest.internal.PreparedContext$runTest$1.invoke(PreparedContext.kt:21)
	at dev.minutest.internal.PreparedContextKt.tryMap(PreparedContext.kt:61)
	at dev.minutest.internal.PreparedContextKt.access$tryMap(PreparedContext.kt:1)
	at dev.minutest.internal.PreparedContext.runTest(PreparedContext.kt:21)
	at dev.minutest.internal.TestExecutor$andThen$1$runTest$testletForParent$1.invoke(TestExecutor.kt:25)
	at dev.minutest.internal.TestExecutor$andThen$1$runTest$testletForParent$1.invoke(TestExecutor.kt:17)
	at dev.minutest.internal.RootExecutor.runTest(TestExecutor.kt:36)
	at dev.minutest.internal.TestExecutor$andThen$1.runTest(TestExecutor.kt:28)
	at dev.minutest.internal.TestExecutor$andThen$1.runTest(TestExecutor.kt:28)
	at dev.minutest.internal.TestExecutor$DefaultImpls.runTest(TestExecutor.kt:12)
	at dev.minutest.internal.TestExecutor$andThen$1.runTest(TestExecutor.kt:17)
	at dev.minutest.junit.JUnit5MinutestsKt$toDynamicNode$1.execute(JUnit5Minutests.kt:59)

...

My expectation is that

deriveFixture: l2 skipped

would not need to be called since those tests are skipped.

@dmcg
Copy link
Owner

dmcg commented May 22, 2019

Hmm good point.

I'm not sure that there's an easy fix for this one. Because of the nature of the JUnit5 runner, skip is implemented as throwing an exception in the test method execution. In order to start that execution, the chain of fixture setup is invoked.

Does plain old JUnit, instantiate an instance of the test class just in order to not invoke its method I wonder? Probably not, but it has separate discovery and execution phases that the JUnit5 runner, based as it is on Streams, does not.

As you can probably tell by this stream of consciousness, I'm thinking this through. I'll let this issue mull for a while.

@mkobit
Copy link
Author

mkobit commented Jun 5, 2019

I'm not sure that there's an easy fix for this one. Because of the nature of the JUnit5 runner, skip is implemented as throwing an exception in the test method execution. In order to start that execution, the chain of fixture setup is invoked.

That makes sense. I do like the current flexibility of minutest in that it just uses the dynamic test feature of JUnit under the hood, but dynamic tests don't support the various extensions APIs yet (junit-team/junit5#378). @TestTemplate or a test engine could possibly solve this, but that is an entirely different track of code to support. I wonder if this issue could be solved if junit-team/junit5#378 was complete (just pondering).

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