mvysny / karibu-testing

Vaadin Server-Side Browserless Containerless Unit Testing
Apache License 2.0
105 stars 14 forks source link

UI.accessSynchronously() is not executed #79

Closed hfazai closed 2 years ago

hfazai commented 2 years ago

I have a code executed in a thread which creates a DatePicker:

Thread {
  val datePicker = DatePicker()
  ...
}.start()

To create a DatePicker in a thread you need to lock the session because it gets the locale from the UI. In DataPicker constructor we find this line:

setLocale(UI.getCurrent().getLocale());

So to lock the session I had to do this:

Thread {
  var datePicker: DatePicker? = null
  ui.accessSynchronously {
    datePicker = DatePicker()
  }

  // Do things with datePicker... (e.g. add it to the main layout)
}.start()
  1. Is it right to call UI.getCurrent() in DatePicker's constructor? Because we lock the session when we update the UI from a threa, and here I'm just creating an object. Maybe we can request to improve it :)
  2. Is my workaround good to fix the issue?
  3. It works fine when I run the app but karibu-testing is not executing the block inside accessSynchronously
mvysny commented 2 years ago

Hi, it is a programming error to create or manipulate Vaadin components outside of the UI thread. You have to do that either outside of the custom thread, or via one of the ui.access* methods.

Karibu-Testing correctly runs the accessSynchronously() block - there are tests for that. Therefore perhaps the ui is null, which causes the ui.accessSynchronously to fail with NPE, which is then silently consumed after the thread ends. Try wrapping the statement with try-catch and print the exception.

hfazai commented 2 years ago

Hi Martin,

That's the first thing I did. I used ui.access but datePicker is intialized later which leads to NPE.

it is a programming error to create or manipulate Vaadin components outside of the UI thread

In this case I will initialize it in the UI thread. And that will require me to make too much changes in the code :D

Karibu-Testing correctly runs the accessSynchronously() block - there are tests for that. Therefore perhaps the ui is null, which causes the ui.accessSynchronously to fail with NPE, which is then silently consumed after the thread ends. Try wrapping the statement with try-catch and print the exception.

I tried that and no error was printed. And I made a breakpoint, and the debugger didn't enter into the block.

I created this simple test and it's failing:

@Test
  fun `test`() {
    _expectOne<Button>()
  }

Code:

@Route("")
class VApplication : VerticalLayout() {

  override fun onAttach(attachEvent: AttachEvent) {
    Thread {
      attachEvent.ui.accessSynchronously {
        add(Button("foo"))
      }
    }.start()
  }

}

Maybe accessSynchronously shouldn't be called from a thread?

mvysny commented 2 years ago

The thread is started but it's not done executing. You need to join() on the thread, to make sure that the thread terminates before you assert on the button.

hfazai commented 2 years ago
override fun onAttach(attachEvent: AttachEvent) {
    val t = Thread {
      attachEvent.ui.accessSynchronously {
        add(Button("foo"))
      }
    }
    t.start()
    t.join()
  }

It keeps running indefinitely.

mvysny commented 2 years ago

Uh-oh, join() was probably not a good idea since that code forms a deadlock. The thread can't proceed since the testing thread holds the UI lock. Anyways, please read https://github.com/mvysny/karibu-testing/tree/master/karibu-testing-v10#testing-asynchronous-application for more information.

hfazai commented 2 years ago

Ok thanks Martin.

I'm already using Karibu-Testing to test asynchronous updates and it works fine.

join() was probably not a good idea since that code forms a deadlock.

In which case that forms a deadlock? I'm running the same app and the button is added.

hfazai commented 2 years ago

I resolved this by calling the factory that creates the Vaadin components in ui.access block.

Thanks @mvysny for the answers.

mvysny commented 2 years ago

In which case that forms a deadlock? I'm running the same app and the button is added.

Sure, let me explain. Let's take a look at

override fun onAttach(attachEvent: AttachEvent) {
    val t = Thread {
      attachEvent.ui.accessSynchronously {
        add(Button("foo"))
      }
    }
    t.start()
    t.join()
  }

The testing code attaches a button which in turn calls onAttach(). The onAttach() starts the thread and joins it, awaiting for it to terminate. The very important information is that the testing code is holding the UI lock.

The thread calls the accessSynchronously() function which tries to grab the UI lock, waiting until the lock becomes ready. However, the lock is held by another thread - the thread which runs the test code.

There are multiple of possible solutions; the easiest way is to replace t.join() with Thread.sleep(200) or similar. Yes, it's a bit fragile and possibly ugly, but it may get the job done. Alternatively you could release the UI lock, call t.join(), then re-acquire the lock afterwards, but things start to get complicated, as it unfortunately always seems to be with the threads...

@hfazai if you agree that the question has been answered, please feel free to close this ticket :+1:

hfazai commented 2 years ago

I finally used access instead of accessSynchronously to avoid deadlocks as mentioned in accessSynchronously docs.

Yes this provides an answer to my question. Thanks for the explanation :+1: