Coder Social home page Coder Social logo

Testing of extension methods about kotter HOT 25 CLOSED

Nereboss avatar Nereboss commented on September 27, 2024
Testing of extension methods

from kotter.

Comments (25)

Nereboss avatar Nereboss commented on September 27, 2024 1

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.

from kotter.

Nereboss avatar Nereboss commented on September 27, 2024

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

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

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

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

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.

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

@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.

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

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.

from kotter.

Nereboss avatar Nereboss commented on September 27, 2024

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.

from kotter.

Nereboss avatar Nereboss commented on September 27, 2024

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.

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

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.

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

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() }
}

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

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.

from kotter.

Nereboss avatar Nereboss commented on September 27, 2024

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!

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

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

from kotter.

Nereboss avatar Nereboss commented on September 27, 2024

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

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

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.

from kotter.

Nereboss avatar Nereboss commented on September 27, 2024

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 :)

from kotter.

Nereboss avatar Nereboss commented on September 27, 2024

@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!

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

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

from kotter.

Nereboss avatar Nereboss commented on September 27, 2024

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.

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

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.

from kotter.

Nereboss avatar Nereboss commented on September 27, 2024

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.

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

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...

from kotter.

bitspittle avatar bitspittle commented on September 27, 2024

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).

from kotter.

Nereboss avatar Nereboss commented on September 27, 2024

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

from kotter.

Nereboss avatar Nereboss commented on September 27, 2024

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.

from kotter.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.