Testing of extension methods

[Nereboss]

Is your feature request related to a problem? Please describe.

When writing extension functions, i want to be able to properly test them.

Describe the solution you'd like
Extension of kotterx to better support testing of whole methods that include section and run parts. I believe this is important to make kotter a more viable choice for larger codebases.

Describe alternatives you've considered
I tried simulating keypresses to test user input for the method using the sendKeys function of kotterx and starting it in a coroutine to fire after the method to test was started but i failed to achieve any results.

Additional context
Here is the code for one of the methods i wrote that i want to test:

fun Session.promptConfirm(message: String): Boolean {
    var result = true
    var choice by liveVarOf(true)
    section {
        drawConfirm(message, choice)
    }.runUntilSignal {
        onKeyPressed {
            println(key)
            when (key) {
                Keys.LEFT -> choice = true
                Keys.RIGHT -> choice = false
                Keys.ENTER -> { result = choice; signal() }
            }
        }
    }
    return result
}

private fun MainRenderScope.drawConfirm(message: String, choice: Boolean): Unit {
    green { text("? ") }; text(message)
    black(isBright = true) { textLine("  (use arrow keys to change selection)") }
    if (choice) {
        text("> "); cyan { text("[Yes]") }; textLine(" No ")
    } else {
        text(">  Yes "); cyan { textLine("[No]") }
    }
}

This snippet changes based on user input and only returns once the user pressed enter, so i would expect that i can use functionality of kotterx to simulate these keypresses to test my method.
Im not sure if this is already possible as there there are methods to simulate keypresses

@Test
    fun `should return true when no arrow keys were pressed`() {
        var result = false
        testSession {
            runBlocking {
                launch {
                    delay(1000)
                    println("key sent")
                    it.sendKey(13)
                    println("check for blocking")
                }
                result = myPromptConfirm("Unimportant test message")
            }
        }
        Assertions.assertTrue(result)
  }

This was my attempt at simulating pressing the enter key but the sendKey method kept blocking. From what i have concluded so far, kotterx already gives great tools to test everything that is being rendered, which can be applied for extension methods too if the method extends for example MainRenderScope. But how to test the logic that happens in a run block (like the user input in the example above) is either currently not possible or not documented enough.

[Nereboss]

@bitspittle friendly poke :)
What is your opinion about this?

[bitspittle]

Thanks for the poke. I have to update my email settings, GitHub issues keep getting swallowed.

[bitspittle]

As for not documented enough, totally agreed. See #89

It's probably worth adding a detailed testing README to that module soon and closing that bug.

[bitspittle]

@Nereboss

This is a WIP README (and in fact, after tomorrow, this link will probably stop working), but I'm mostly done at this point. I'm going to rest for the night and look at things tomorrow with fresh eyes.

https://github.com/varabyte/kotter/blob/tmp%2Bkotter-test-readme/kotterx/kotter-test-support/README.md

While it doesn't directly address your question, you are welcome to take a sneak peak and let me know if it is helpful.

[bitspittle]

Apologies for the late reply for actually answering your original question. I realize I didn't really understand it until about an hour ago.

So, for some reason that I'm not sure about, I needed to create a new thread for sending the keys to stop your code from getting blocked. This works:

 @Test
 fun `should return true when no arrow keys were pressed`() {
     var result = false
     testSession { terminal ->
         CoroutineScope(Dispatchers.Default).launch {
             delay(1000)
             terminal.type(Ansi.CtrlChars.ENTER)
         }
         result = promptConfirm("Unimportant test message")
     }
     Assertions.assertTrue(result)
 }

Coroutines... they're awesome but I still don't get why some situations block.

---

I don't like the delay in that approach, though. That feels fragile. Also, it makes your test take a second to run! If you have 100 such tests, your test suite will take over a minute to run...

So one approach to consider is adding an event callback in your promptConfirm method. This is pretty common way to turn untestable code into testable code, when push comes to shove:

fun Session.promptConfirm(message: String, onInputReady: suspend () -> Unit = {}): Boolean {
    section { /* ... */  }.runUntilSignal {
        onKeyPressed { /* ... */ }
        onInputReady()
    }
    return result
}

 @Test
 fun `should return true when no arrow keys were pressed`() {
     var result = false
     testSession { terminal ->
         result = promptConfirm("Unimportant test message", onInputReady = {
             terminal.type(Ansi.CtrlChars.ENTER)
         })
     }
    Assertions.assertTrue(result)
 }

This is how I would do it.

---

A bonus suggestion, which I do not recommend but will include here for educational purposes, is to point out that the session data property allows users to register a listener for when Kotter lifecycles end. You can use this to listen to the end of a render pass and send your terminal keypress at that point.

Note that this is potentially fragile because there's no guarantee that input will be hooked up by the time the first render finishes, but I tested things locally in a long-running loop, and it seemed to work reliably.

@Test
fun `should return true when no arrow keys were pressed`() {
    var result = false
    testSession { terminal ->
        data.onLifecycleDeactivated {
            if (lifecycle == MainRenderScope.Lifecycle) {
                removeListener = true
                CoroutineScope(Dispatchers.Default).launch {
                    terminal.type(Ansi.CtrlChars.ENTER)
                }
            }
        }
        result = promptConfirm("Unimportant test message")
    }
    Assertions.assertTrue(result)
}

Still, not a big fan of this approach because the relationship between a render pass finishing and the run block setting up its input is indirect. Also, the lifecycles of Kotter can be pretty mysterious to people who aren't familiar with its implementation, which can make the test hard to understand.

---

Maybe there's another way to accomplish this that I'm missing. Or maybe Kotter needs an additional hook so that tests can get notified when input is ready to be typed.

Since there are a few ways here to work around the issue, it's probably not a priority to find a fix at the moment, but maybe if I sleep on it, I'll think of a better way.

[Nereboss]

Thank you for the detailed answer!

I read through the WIP readme and it was really helpful to better understand the testing environment. I dont know how common the use case of writing extension methods is but it could maybe use a small section with an example stating that they can be tested the same way as built in functions.

I agree that delay is not a good fit for these tests. For now i will use the approach you proposed using an event callback. I dont really like that i need to change the code just to test it but it seems to be the best approach right now.

I do think though that Kotter could use a way to more comfortably test user input as handling user input is one of its main features.

For example something like this:

@Test
    fun `should return true when no arrow keys were pressed() {
        var result = false
        testSession { terminal ->
            executeUntilInputRequired {
                result = promptConfirm("test message")
            }.sendInput {
                terminal.type(Ansi.CtrlChars.ENTER)
            }
            terminal.assertMatches { textLine( /* ... */ ) }
        }
        Assertions.assertTrue(result)
    }

Where the executeUntilInputRequired renders everything to the point where input() is called in the code and the sendInput block provides a more user friendly way to simulate user input than the callbacks you mention as a workaround.

I do not know if something like that is possible with the scope structure (i dont think i understood it all well enough to judge) or if this is even something you consider necessary for kotterx.

[Nereboss]

Another small thing that i noticed from reading the readme is that the assertMatches function could use an option to ignore formatting and/or ANSI characters. (This could be its own issue but as it is connected to this ill write it here)
I assume it is often the case that formatting stays static and only content is changed so an assert like this:

terminal.assertMatches {
    bold {
            green { text("? ") }; text(testMessage)
            black(isBright = true) { textLine("  $defaultConfirmHint") }
        }
        text("> "); text(" Yes "); cyan { textLine("[No]") }
    }

Could be changed to:

terminal.assertMatches(ignoreFormatting = true) {
    textLine("? $testMessage")
    textLine(defaultConfirmHint)
    textLine(">  Yes [No]")
}

This would make tests a lot more readable at the cost of not covering formatting.

[bitspittle]

I dont know how common the use case of writing extension methods is but it could maybe use a small section with an example stating that they can be tested the same way as built in functions.

I'll add a section to the README on how to write a test for a realistic scenario similar inspired by what you shared here.

I dont really like that i need to change the code just to test it but it seems to be the best approach right now.

It's a pretty common thing with testing -- how to poke holes into classes and methods to make them testable but in a way that's not too hacky. For classes, usually this is done with dependency injection -- a fancy way to say you can pass in logic implemented externally but with a useful default implementation for production code. Of course, you can also expose event callbacks (like I've done here) which are useful for tests and not so much for public code (but otherwise harmless, minus a bit of API complexity).

Note that you have the option of creating an internal method you can delegate to:

// Extra params useful for testing, but not for users!
internal fun Session.promptConfirm(message: String, onInputReady: suspend () -> Unit): Boolean { ... }

fun Session.promptConfirm(message: String) = promptConfirm(message, onInputReady = {})

I do think though that Kotter could use a way to more comfortably test user input as handling user input is one of its main features.

Unfortunately, I believe the design of Kotter, powerful in many ways, makes your suggestion hard to do. The core of Kotter is really lean and doesn't even know about the input method -- that's simply an extension on top of its foundation, which you yourself could write if Kotter didn't provide it. All Kotter knows is that it can render stuff if you ask it to, and that it is associated with a terminal which may or may not be receiving input at any time.

(When using a Kotter program, this isn't a problem, because the user won't type anything until they see the screen, at which point the render has already happened. If for some reason they typed so fast that input wasn't ready, they would see that the screen didn't change, and they would just press the key again).

It's hard to add hooks into Kotter as well, because so much power is given to the user. A user could start a run block, register an onKeyPressed handler, put the run block to sleep (using delay) for a minute, and then continue running code, if they wanted to.

It's not really clear where I would add a hook in for that. However, you, as the user doing that fancy delay thing, could easily create a hook for yourself, which I think is still what I'd recommend here after sleeping on it.

[bitspittle]

This would make tests a lot more readable at the cost of not covering formatting.

I agree, this would be a nice addition to the testing library.

I would not use assertMatches for that, because it's weird you could do something like:

assertMatches(ignoreFormatting = true) {
   green { textLine("???") }
}

However, I am thinking about this:

assertThat(terminal.resolveRerenders().stripFormatting()).containsExactly(
   "? $testMessage",
   defaultConfirmHint,
   ">  Yes [No]",
   "",
).inOrder()

I already tested this a little bit locally, and will likely upload a snapshot in the next day or two after giving it more time to bake, but if you want to have the code now, you can add this in your own project:

fun String.stripFormatting(): String {
    val textPtr = TextPtr(this)

    return buildString {
        while (textPtr.remainingLength > 0) {
            when (val c = textPtr.currChar) {
                Ansi.CtrlChars.ESC -> {
                    textPtr.increment()
                    // As a side effect, the `parts` parsing code consumes the text pointer
                    when (textPtr.currChar) {
                        Ansi.EscSeq.CSI -> { textPtr.increment(); Ansi.Csi.Code.parts(textPtr) }
                        Ansi.EscSeq.OSC -> { textPtr.increment(); Ansi.Osc.Code.parts(textPtr) }
                    }
                }
                else -> append(c)
            }
            textPtr.increment()
        }
    }
}

fun Iterable<String>.stripFormatting(): List<String> {
    return this.map { it.stripFormatting() }
}

[Nereboss]

Thanks again for explaining it all, as ive never developed anything comparable to kotter myself, a lot of these concepts are new to me. I did not think about using an internal function for the callbacks, that looks like a very good solution!

For the stripFormatting: I will continue working on my tests on monday, if you want to i can test the snapshot again.

[bitspittle]

Heads up that https://github.com/varabyte/kotter/blob/tmp%2Bkotter-test-readme/kotterx/kotter-test-support/README.md has been updated and is probably close to a final state I like to call "good enough".

It's essentially the same as what you already saw (with some wording cleaned up after reviewing it with fresh eyes). However, I added https://github.com/varabyte/kotter/tree/tmp%2Bkotter-test-readme/kotterx/kotter-test-support#a-realistic-scenario inspired by your suggestion. No pressure, but if you check it out, let me know if it works OK as an example.

My plan is to copy these docs over to the main branch (so that people can see them right away). After that, I'll add the stripFormatting method to the test library and upload a 1.1.3 snapshot.

I'm also thinking of adding a terminal.press(...) helper method that just takes Kotter keys directly. That would allow you to simplify the code in my example, from:

terminal.sendCode(Ansi.Csi.Codes.Keys.DOWN)
terminal.type(Ansi.CtrlChars.ENTER)

to just this:

terminal.press(Keys.DOWN)
terminal.press(Keys.ENTER)

Hopefully that might also help you out.

[Nereboss]

I took a look at the new section and it looks really good, very helpful for someone who is new at testing kotter.
I agree that the terminal.press helper is a nice addition. When I first used type and sendCode i was confused why it didnt take Kotter keys, so this feels very natural and will certainly be helpful!

[bitspittle]

OK, 1.1.3-SNAPSHOT should be up.

Don't forget you need to add a special repository to access snapshots. See also: https://github.com/varabyte/kotter/tree/1.1.3#testing-snapshots

[Nereboss]

Tested both the stripFormatting and terminal.press methods from the snapshot and they work as intended. Was anything else added that i should test?

[bitspittle]

Awesome, no, that's everything! Thanks for confirming.

I think at this point we can close this issue? Let me know if there's anything else that you feel needs to be resolved.

[Nereboss]

Everything concerning this issue has been resolved, thank you! But theres one more thing i want to ask you because ive been trying to find a good solution for a specific test case but just cant get it working.

I have an extension function where the user is asked to input something that is evaluated to be valid or invalid using the inputValdator (irrelevant for this case). But when the file is invalid, i do not accept the input and display a warning. I want to test that this warning is displayed correctly but the challenge for me is that, as the input is not accepted, the function is blocking, waiting for a valid input. Here is the code of the method:

private fun MainRenderScope.drawInput(
        message: String,
        hint: String,
        isInputValid: Boolean,
        allowEmptyInput: Boolean,
        invalidInputMessage: String,
        lastUserInput: String
) {
    bold {
        green { text("? ") }; text(message)
        if (isInputValid) {
            black(isBright = true) { textLine(if (allowEmptyInput) "  empty input is allowed" else "") }
        } else {        //this next line is the one i want to test
            red { textLine(if (lastUserInput.isEmpty()) "  Empty input is not allowed!" else "  $invalidInputMessage") }
        }
    }
    text("> "); input(Completions(hint), initialText = "")
}

internal fun Session.myPromptInput(
        message: String,
        hint: String = "",
        allowEmptyInput: Boolean = false,
        invalidInputMessage: String = "Input is invalid!",
        inputValidator: (String) -> Boolean = { true },
        onInputReady: suspend () -> Unit = {}
): String {
    var lastUserInput = ""
    var hintText = hint
    var isInputValid by liveVarOf(true)
    section {
        drawInput(message, hintText, isInputValid, allowEmptyInput, invalidInputMessage, lastUserInput)
    }.runUntilSignal {
        onInputChanged { isInputValid = true }
        onInputEntered {
            if ((allowEmptyInput && input.isEmpty()) || (inputValidator(input) && input.isNotEmpty())) {
                isInputValid = true
                hintText = ""
                signal()
            } else {
                isInputValid = false
            }
            lastUserInput = input
        }
        onInputReady()
    }
    return lastUserInput
}

My first idea was something similar to this:

@Test
    fun `should not accept input and display error when input is empty but empty input is not allowed`() {
        var result = "not empty"

        testSession { terminal ->
            result = myPromptInput(testMessage, allowEmptyInput = false,
                    onInputReady = {
                        terminal.press(Keys.ENTER)

                        terminal.assertMatches { textLine(/* expected result */) }
                   }
            )
        }
        assertThat(result).isEqualTo("")
    }

But the assertMatches does not include the rerender. I can also not call blockUntilRenderMatches here, as the OnInputReady callback is not of a type that would allow it.
I dont know if this is enough information to understand the issue but i still wanted to get your opinion on if there is an easy way to test this :)

[Nereboss]

@bitspittle actually, i found one last thing that could be useful for testing. The blockUntilRenderMatches function could include an option similar to the stripFormatting, so that im able to test for only the text and not all formatting in every test.

An example for such a usecase would be the following:

@Test
    fun `should not accept empty input and display warning message when empty input is not allowed`() {
        testSession { terminal ->
            myPromptInput(testMessage, allowEmptyInput = false,
                onInputReady = {
                    terminal.press(Keys.ENTER)
                    blockUntilRenderMatches(terminal) { /* expected result without formatting */ }

                    terminal.type("irrelevant non empty input")
                    terminal.press(Keys.ENTER)
                }
            )
        }
    }

This tests checks if a warning is displayed when the user input did not fit certain criteria and is therefore rejected (in our case input is empty but it is not allowed to be).

Other than that the readme and the methods you wrote for the snapshot have really helped with testing and as soon as 1.1.3 releases i can integrate kotter into the project im working on!

[bitspittle]

Sorry that I missed your response earlier from 3 weeks ago. I've updated my gmail settings so hopefully I won't miss things like this as much anymore.

I think in the "input valid" case, it sounds like you are "testing your UI" which honestly is normally not recommended. You shouldn't have tests fail just because you decide to reword some text in the future.

That said, I'm assuming you could have done something like:

internal fun Session.myPromptInput(
   ...
   onInputReady: suspend RunScope.() -> Unit = {}

if you want to use blockUntilRenderMatches from the callback.

---

For your last comment, note that blockUntilRenderMatches delegates to blockUntilRenderWhen which is what you want to be using here:

Something like

onInputReady = {
  blockUntilRenderWhen {
    terminal.buffer.stripFormatting() == "...."
  }
}

I'll consider adding docs for that in the README

[Nereboss]

I ended up solving the 'UI testing' using the RunScope like you suggested.

---

For the blockUntilRenderWhen: When i tested it out in my example, i needed to surround it with try catch, otherwise the test always went through. Even something like this, which i feel like should throw an error:

@Test
    fun `should not accept empty input and display warning message when empty input is not allowed`() {
        testSession { terminal ->
            myPromptInput(testMessage, allowEmptyInput = false,
                onInputReady = {
                    terminal.press(Keys.ENTER)
                    blockUntilRenderWhen {
                        false
                    }
                    terminal.type("irrelevant non empty input")
                    terminal.press(Keys.ENTER)
                }
            )
        }
    }

Not sure if its a bug with the method or intended but i find it unintuitive nonetheless.

I dont think this specific example needs to be in the readme but i the stripFormatting itself should be mentioned once its fully released.

[bitspittle]

blockUntilRenderWhen takes a timeout duration which defaults to one second. I'm guessing that's triggering and throwing an exception. Are you not seeing the exception?

I bet if you did blockUntilRenderWhen(Duration.INFINITE) { false } your test would freeze.

BTW, the README already mentions stripFormatting, so I'm a little confused by your final statement.

FYI I'm planning to release 1.1.3 at some point soon, where the base library didn't change at all but the testing library improvements will be made public.

[Nereboss]

When i run the above mentioned example, i would have expected the test to fail because the condition inside the blockUntilRenderWhen is always false which should lead to a TimeoutCancellationException being thrown. I assumed this exception would lead to the test failing which does not seem to be the case.

When i copy the code from the blockUntilRenderMatches function to catch that exception, everything works as expected so the exception is being thrown.

Im just confused why that didn't lead to the test failing but this could also be a problem of my kotlin knowledge and not of your function ^^
What made me think it might be a bug is that when i use blockUntilRenderMatches, on something that doesn't match, the test fails as expected. But when i use blockUntilRenderWhen, the test always passes.

I tested it with Duration infinite and it did freeze as expected so you were right about that one.

Sorry about the README, i was looking at an outdated version and did not see that stripFormatting is mentioned now.

[bitspittle]

I reproduced what you're seeing. I'll see if I can fix it.

What's very odd is test #1 passes but test #2 fails:

    @Test
    fun throwtest1() = testSession { terminal ->
        section {}.runUntilSignal {
            throw CancellationException("✅")
        }
    }

    @Test
    fun throwtest2() = testSession { terminal ->
        section {}.runUntilSignal {
            throw IllegaleStateException("💀")
        }
    }

A CancellationException is an IllegalStateException except it gets swallowed by coroutine machinery, so that's likely what's happening here (to a coroutine, a "cancel exception" is just a signal to stop, not a reason to explode).

The easiest fix is probably catching the timeout exception and rethrowing it as a non-coroutine exception...

[bitspittle]

A new 1.1.3-SNAPSHOT is up which should "fix" the timeout exception ("fix" in that your test should now fail instead of succeeding quietly).

[Nereboss]

I don't see a new snapshot but i copied the changes manually and now the tests behave like expected 👍

The only nitpick would be that blockUntilRenderWhen does not give a nicely formatted error message like assertMatches does but thats just a minor detail

[Nereboss]

Have you had any experience with mocking kotter functions? Im currently testing if a 'run' through my program consisting of multiple questions returns the expected result. Currently the tests always open a virtual terminal (probably because gradle is already consuming the interactivity) so im trying to get around that interactivity by simply mocking the extension functions to simply return a value.
It could also be useful to test functions that contain the input function as the return could just be mocked instead of a combination of type and press.

[Nereboss]

Thank you very much for your feedback on the matter! @bitspittle (ping in case the message would get lost otherwise ^^)

I discussed it with the team and we ended up agreeing on a lot of your points so we will reduce mocking throughout the whole project.

I also found out that the cause for the virtual terminal was opening a session inside of a test session rather than gradle. So through some refactoring i was able to test everything without the need for mocks.

You were also right about the XY problem. I hadn't heard of it before but it helped me to stay focused on the original problems since.

Do you have plans to release 1.3? Im currently using the snapshot but thats missing the fix for the timeout exception.

[bitspittle]

Wow, I influenced a team to push back on mocking?? You made my day 🤗 Thanks all for listening and thinking about it deeply!

Apologies for the delay on releasing 1.3 -- I'm actually preparing for a talk at Droidcon, which I'll need to give in almost exactly one month from now. The problem is that I need to finish some huge changes in Kobweb first, things that have been long planned that will affect the talk significantly. At this point, they are almost done -- but it's been like a week or two straight of wrangling with really scary technical challenges. Once done, my reward is I have a bunch of documentation to update, and then I actually have to prepare the talk....

So the 1.3 release fell off my radar. I'll try to find time in the next few days to push it out, but just be aware I'm like in the middle of a marathon so I might forget. Do feel free to ping again in a week to get my attention in that case.

[Nereboss]

That really does sound like a marathon...

Is it less effort to upload a snapshot than making the release?
For us it doesn't really matter if we use a snapshot for a while. Only problem being that the timeout exception fixes are not in the current snapshot.
If a snapshot or a release are the same effort, i'd say focus on your talk for now! That sounds more important and we will find a way to work with the current snapshot :)

[bitspittle]

Uploading a snapshot is easy! A full release isn't necessarily difficult (unless the maven servers are having a bad day, which is not rare), but it's a lot more scary because it's irreversible. So if something goes wrong, I may need to spend a few hours scrambling to do another release on top of it, which worst case can be a chain reaction from hell.

I thought I did upload a snapshot for 1.1.3, but just in case, I just pushed another one out this morning with latest. Let me know if you can pull it down.

[Nereboss]

You did upload a new 1.1.3 snapshot. My IDE somehow didn't update my local version but it works now. So from our side the new release can wait until after your marathon.

I did however open a new issue (sorry ^^) because some of our tests are flaky and it might be a bug in kotters testing library.