detect when idle

[zigdon]

In the testing code, we include a sleep and hope all the gui events have been handled. It would make tests simpler (and more deterministic) if there was an API to call and return when all pending events have been handled.

[mjarkk]

Agree, Just looked into the code a bit but i don't see a clean solution yet for solving this yet. I'm myself very busy so if anyone wants to look into solving this cleanly would be very appreciated :)

[zigdon]

We have a bunch of discussion in #101, but as we're drifting away from that particular PR and discussing the design, it might make sense to have that discussion here.

At it's core, I was bothered by having the sleep in testing_test.go, and thought we should have a more deterministic way to run a test. In the discussion referenced above, we've suggested a couple of ways to go about this, with I think the current best option being creating some sort of 'sync' event, that can ensure that all preceding events have been processed.

I tried to do something like that at some point, but was foiled by flush() triggering a screen redraw. If we can avoid that, perhaps this problem goes away:

var wg sync.WaitGroup
wg.Add(1)
g.Update(func(*Gui) error {
  wg.Done()
  return nil
})
wg.Wait()

The only concern I see if we do that is that this triggers a userEvent, so it can't force a sync with any gEvents. If we created a dedicated sync event though, we could ensure its callback doesn't get triggered until consumeEvents is actually complete.

What do y'all think?

[mjarkk]

Just dumping my thoughts here.

When testing a UI i presume you would want to send keys and check if the content changed on screen for example:

1. Press "A"
2. Check if modal pops up
3. Press "B"
4. Check if a input modal pops up
5. Press "ABCDE"
6. Check if the input of the modal is "ABCDE"
7. Press "ESC"
8. Check if the modal has disappeared
9. Press "CTRL+C"
10. End of test

With the current and suggested code we will have a lot of timeouts in our testing code but do we want to write tests that way? I've looked a bit into other libraries and it seems like this is happening often:

1. The event is fired
2. Wait for the UI to update (it's up to the library user if they want to wait for this)

In most javascript frameworks this is usually expressed as the next UI "tick" and you can wait for it by prefixing the event method with await or calling the tick method manually.

I quite like this way of writing tests in javascript:

// Do not wait for a UI update
fireEvent.click(incrementElement)

// Wait for the UI update
await fireEvent.click(incrementElement)

It's very clean and it's up to you if you want to wait for the "event to finish" as in the ui has updated based on user input.
What if we would do the same thing by returning a channel:

// Do not wait for a UI update
testingScreen.SendKey(KeyCtrlC)

// Wait for the UI update
<-testingScreen.SendKey(KeyCtrlC)

[dankox]

So after reading thru it and thinking about it, the WaitIdle approach is really not good and not suitable for this specific situation where user just wants to test if the pressed key changed the screen accordingly.

@mjarkk what you said makes sense and now I see what's the actual problem.

I have a solution in mind, I will quickly create POC and let you guys know about it and we can discuss if it's usable or not or if it has any edge cases.

[dankox]

I've created a POC for synchronized keys when testing: https://github.com/dankox/gocui/tree/feature/test-sync-keys

The implementation adds new eventTime to the gocui event which is on the same channel as all the other tcell events. This way we know that all the events before this were processed.
Gocui checks for this event and if testing is in place, it will send notification thru that channel which needs to be read for proceeding. It's non-buffered so the synchronization will wait.

There is also a test which is using it with some delay and reading the screen content.

@mjarkk I couldn't implement the version which you proposed with returned channel, because the channel needs to be read everytime when eventTime is used. So the waiting is separate function and is also included in SendKeySync (for simplification).

Let me know guys, what you think.

[mjarkk]

@dankox your suggested testing API looks good to me, that's exactly what i would expect the testing library to work.
I have a few code comments but nothing special that can't be fixed in a PR.

@zigdon Are you happy with this?

[zigdon]

Yeah, that sounds much cleaner than my original proposal. I would love to have the channel api you proposed above, but like you said, it becomes tricky figuring out when to block and when to wait.

Some comments for the code, but those can wait for a PR. I worry there's a case where we might combine multiple sync events into one, and end up hanging.

[dankox]

@zigdon thanks for pointing out that issue, you are right.
I will fix that and add some tests for such case. I'll create the PR later (after that fix), hopefully this evening or during tomorrow.

[dankox]

I've created new PR with some minor update (for parallel sync keys). Feel free to check and comment :)