Differential D22119

[autotests] In internal window test remove spy waits or guard them
ClosedPublic
Actions

Authored by romangg on Jun 27 2019, 12:13 PM.

Details

Reviewers
zzag
Group Reviewers
KWin
Commits
R108:349560a78c47: [autotests] In internal window test remove spy waits or guard them
Summary

The internal window test is failing on CI because the client add spy waits
are not triggered. The signal has been emitted already at this point.

Removing them fixes this (the condition is still checked by subsequent
count verify on the spy) in all but one instance. In this case the wait
needs to be guarded.

Is there a more general approach to it? Always guarding is ugly. Also when
was this test regression introduced? In the past we must have had some
slack until the signal was fired to start the wait call.

Test Plan

Internal window test passes with this patch again.

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
romangg created this revision.Jun 27 2019, 12:13 PM
Restricted Application added a project: KWin. · View Herald TranscriptJun 27 2019, 12:13 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
romangg requested review of this revision.Jun 27 2019, 12:13 PM
zzag added a subscriber: zzag.Jun 27 2019, 12:22 PM

Also when was this test regression introduced?

git bisect?

romangg added a comment.Jun 27 2019, 12:40 PM
In D22119#487232, @zzag wrote:

Also when was this test regression introduced?

git bisect?

I already went back in history some time and test always failed for me. So I thought maybe it's caused by a framework change and/or someone does know directly. Finding out is low priority though.

zzag added a comment.Jun 27 2019, 1:35 PM

So it seems like some events are dispatched when wl_event_loop_dispatch(..., timeout=0) is called.

Glorious backtrace
(lldb) bt                                                                                                                                                                                                                                                                                                                                                               
* thread #1, name = 'testInternalWin', stop reason = breakpoint 1.1                                                                                                                                                                                                                                                                                                     
  * frame #0: 0x00007ffff7dd1df2 libkwin.so.5`KWin::ShellClient::addDamage(this=0x0000555555710ec0, damage=0x00005555557fed60) at shell_client.cpp:537:17                                                                                                                                                                                                               
    frame #1: 0x00007ffff7d1fb5c libkwin.so.5`QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<QRegion const&>, void, void (KWin::Toplevel::*)(QRegion const&)>::call(f=81 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00, o=0x0000555555710ec0, arg=0x00007fffffffc4f0)(QRegion const&), KWin::Toplevel*, void**) at qobjectdefs_impl.h:152:20          
    frame #2: 0x00007ffff7d1fa4d libkwin.so.5`void QtPrivate::FunctionPointer<void (KWin::Toplevel::*)(QRegion const&)>::call<QtPrivate::List<QRegion const&>, void>(f=81 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00, o=0x0000555555710ec0, arg=0x00007fffffffc4f0)(QRegion const&), KWin::Toplevel*, void**) at qobjectdefs_impl.h:185:95                            
    frame #3: 0x00007ffff7d1f95f libkwin.so.5`QtPrivate::QSlotObject<void (KWin::Toplevel::*)(QRegion const&), QtPrivate::List<QRegion const&>, void>::impl(which=1, this_=0x00005555557e5d10, r=0x0000555555710ec0, a=0x00007fffffffc4f0, ret=0x0000000000000000) at qobjectdefs_impl.h:414:49                                                                         
    frame #4: 0x00007ffff3fb0327 libQt5Core.so.5`QMetaObject::activate(QObject*, int, int, void**) [inlined] QtPrivate::QSlotObjectBase::call(a=0x00007fffffffc4f0, r=0x0000555555710ec0, this=0x00005555557e5d10) at qobjectdefs_impl.h:394:57                                                                                                                         
    frame #5: 0x00007ffff3fb030b libQt5Core.so.5`QMetaObject::activate(sender=<unavailable>, signalOffset=<unavailable>, local_signal_index=<unavailable>, argv=0x00007fffffffc4f0) at qobject.cpp:3787                                                                                                                                                                                               
    frame #6: 0x00007ffff3fb0795 libQt5Core.so.5`QMetaObject::activate(sender=0x00005555557e45a0, m=<unavailable>, local_signal_index=0, argv=0x00007fffffffc4f0) at qobject.cpp:3658:13                                                                                                                                                                                
    frame #7: 0x00007ffff545e22e libKF5WaylandServer.so.5`KWayland::Server::SurfaceInterface::damaged(this=0x00005555557e45a0, _t1=0x00005555557fed60) at moc_surface_interface.cpp:375:26                                                                                                                                                                                                            
    frame #8: 0x00007ffff54ed43f libKF5WaylandServer.so.5`KWayland::Server::SurfaceInterface::Private::swapStates(this=0x00005555557fed20, source=0x00005555557fedf8, target=0x00005555557fed60, emitChanged=true) at surface_interface.cpp:473:36                                                                                                                                                    
    frame #9: 0x00007ffff54ed6d0 libKF5WaylandServer.so.5`KWayland::Server::SurfaceInterface::Private::commit(this=0x00005555557fed20) at surface_interface.cpp:518:19                                                                                                                                                                                                                                
    frame #10: 0x00007ffff54ee22c libKF5WaylandServer.so.5`KWayland::Server::SurfaceInterface::Private::commitCallback(client=0x0000555555630f30, resource=0x00005555557a9490) at surface_interface.cpp:684:36                                                                                                                                                                                        
    frame #11: 0x00007fffef2a96d0 libffi.so.6`ffi_call_unix64 + 76                                                                                                                  
    frame #12: 0x00007fffef2a90a0 libffi.so.6`ffi_call + 560                                                                                                                        
    frame #13: 0x00007ffff282282f libwayland-server.so.0`___lldb_unnamed_symbol74$$libwayland-server.so.0 + 447                                                                     
    frame #14: 0x00007ffff281f193 libwayland-server.so.0`___lldb_unnamed_symbol23$$libwayland-server.so.0 + 579                                                                     
    frame #15: 0x00007ffff28207f2 libwayland-server.so.0`wl_event_loop_dispatch + 114                                                                                                                                                                                                                                                                                   
    frame #16: 0x00007ffff5475a0b libKF5WaylandServer.so.5`KWayland::Server::Display::Private::dispatch(this=0x0000555555637ee0) at display.cpp:148:31                                                                                                                                                                                                                  
    frame #17: 0x00007ffff5476053 libKF5WaylandServer.so.5`KWayland::Server::Display::dispatchEvents(this=0x0000555555634140, msecTimeout=0) at display.cpp:220:20                                                                                                                                                                                                                                    
    frame #18: 0x00007ffff7de6dd1 libkwin.so.5`KWin::WaylandServer::dispatch(this=0x000055555562a740) at wayland_server.cpp:616:30                                                                                                                                                                                                                                                                    
    frame #19: 0x00007fffedd508cd KWinQpaPlugin.so`KWin::QPA::BackingStore::flush(this=0x00007fffe800a1b0, window=0x00007fffffffd610, region=0x00007fffffffce00, offset=0x00007fffffffcdf8) at backingstore.cpp:91:30                                                                                                                                                                                 
    frame #20: 0x00007ffff4670883 libQt5Gui.so.5`QBackingStore::flush(this=<unavailable>, region=<unavailable>, window=0x00007fffffffd610, offset=<unavailable>) at qbackingstore.cpp:249:20       
    frame #21: 0x00007ffff4508a59 libQt5Gui.so.5`QRasterWindowPrivate::flush(this=<unavailable>, region=<unavailable>) at qrasterwindow.cpp:92:28                                   
    frame #22: 0x00007ffff4508495 libQt5Gui.so.5`QPaintDeviceWindow::exposeEvent(QExposeEvent*) at qpaintdevicewindow_p.h:105:18                                                    
    frame #23: 0x00007ffff450843e libQt5Gui.so.5`QPaintDeviceWindow::exposeEvent(this=<unavailable>, exposeEvent=<unavailable>) at qpaintdevicewindow.cpp:189                                                                                                                                                                                                           
    frame #24: 0x00007ffff44d98b8 libQt5Gui.so.5`QWindow::event(this=0x00007fffffffd610, ev=<unavailable>) at qwindow.cpp:2327:20                                                   
    frame #25: 0x00007ffff4508534 libQt5Gui.so.5`QPaintDeviceWindow::event(this=<unavailable>, event=<unavailable>) at qpaintdevicewindow.cpp:206:26                                               
    frame #26: 0x000055555556b2bf testInternalWindow_waylandonly`KWin::HelperWindow::event(this=0x00007fffffffd610, event=0x00007fffffffd2b0) at internal_window.cpp:144:32                        
    frame #27: 0x00007ffff4b0504b libQt5Widgets.so.5`QApplicationPrivate::notify_helper(this=<unavailable>, receiver=0x00007fffffffd610, e=0x00007fffffffd2b0) at qapplication.cpp:3740:31         
    frame #28: 0x00007ffff4b0ca53 libQt5Widgets.so.5`QApplication::notify(this=0x00007fffffffe050, receiver=0x00007fffffffd610, e=0x00007fffffffd2b0) at qapplication.cpp:3096:31   
    frame #29: 0x00007ffff3f7c16d libQt5Core.so.5`QCoreApplication::notifyInternal2(receiver=0x00007fffffffd610, event=0x00007fffffffd2b0) at qcoreapplication.cpp:1060:24          
    frame #30: 0x00007ffff3f7c3e6 libQt5Core.so.5`QCoreApplication::sendSpontaneousEvent(receiver=<unavailable>, event=<unavailable>) at qcoreapplication.cpp:1467:27               
    frame #31: 0x00007ffff44ce167 libQt5Gui.so.5`QGuiApplicationPrivate::processExposeEvent(e=<unavailable>) at qguiapplication.cpp:3077:43                                         
    frame #32: 0x00007ffff44ce33b libQt5Gui.so.5`QGuiApplicationPrivate::processWindowSystemEvent(e=0x00007fffdc02e810) at qguiapplication.cpp:1911:51                              
    frame #33: 0x00007ffff44a2805 libQt5Gui.so.5`QWindowSystemInterface::sendWindowSystemEvents(flags=(i = 0)) at qwindowsysteminterface.cpp:1148:61                                
    frame #34: 0x00007ffff44a2ba0 libQt5Gui.so.5`QWindowSystemInterface::flushWindowSystemEvents(flags=(i = 0)) at qwindowsysteminterface.cpp:1112:31                               
    frame #35: 0x00007ffff44bd3d9 libQt5Gui.so.5`QPlatformWindow::setVisible(this=0x00005555558045b0, visible=<unavailable>) at qplatformwindow.cpp:189:52                          
    frame #36: 0x00007fffedd5740e KWinQpaPlugin.so`KWin::QPA::Window::setVisible(this=0x00005555558045b0, visible=true) at window.cpp:73:32                                         
    frame #37: 0x00007ffff44d949f libQt5Gui.so.5`QWindowPrivate::setVisible(this=0x000055555577d830, visible=<unavailable>) at qwindow.cpp:402:35                                   
    frame #38: 0x00007ffff44d19ce libQt5Gui.so.5`QWindow::setVisible(this=<unavailable>, visible=<unavailable>) at qwindow.cpp:612:18                                                                                                                                                                                                                                   
    frame #39: 0x00007ffff44d42e1 libQt5Gui.so.5`QWindow::showNormal(this=0x00007fffffffd610) at qwindow.cpp:2142:15                                                                                                                                                                                                                                                                                  
    frame #40: 0x00007ffff44d5d9e libQt5Gui.so.5`QWindow::show(this=0x00007fffffffd610) at qwindow.cpp:2070:19                                                                                                                                                                                                                                                                                        
    frame #41: 0x000055555556bc9d testInternalWindow_waylandonly`KWin::InternalWindowTest::testEnterLeave(this=0x00007fffffffe040) at internal_window.cpp:224:13                                                                                                                                                                                                                                      
    frame #42: 0x00005555555782c0 testInternalWindow_waylandonly`KWin::InternalWindowTest::qt_static_metacall(_o=0x00007fffffffe040, _c=InvokeMetaMethod, _id=3, _a=0x00007fffffffd930) at internal_window.moc:155:35                                                                                                                                                                                 
    frame #43: 0x00007ffff3f8b256 libQt5Core.so.5`QMetaMethod::invoke(this=0x000055555562f930, object=0x00007fffffffe040, connectionType=DirectConnection, returnValue=<unavailable>, val0=<unavailable>, val1=<unavailable>, val2=<unavailable>, val3=<unavailable>, val4=<unavailable>, val5=<unavailable>, val6=<unavailable>, val7=<unavailable>, val8=<unavailable>, val9=<unavailable>) const at qmetaobject.cpp:230
1:25                                                                                                                                                                                                                                                                                                                                                                                                  
    frame #44: 0x00007ffff7a3dff1 libQt5Test.so.5`QTest::TestMethods::invokeTestOnData(int) const [inlined] QMetaMethod::invoke(val9=<unavailable>, val8=<unavailable>, val7=<unavailable>, val6=<unavailable>, val5=<unavailable>, val4=<unavailable>, val3=<unavailable>, val2=<unavailable>, val1=(_data = 0x0000000000000000, _name = <no value available>), val0=<unavailable>, connectionType=DirectConnection, obje
ct=<unavailable>, this=<unavailable>) const at qmetaobject.h:122:22                                                                                                                                                                                                                                                                                                                                   
    frame #45: 0x00007ffff7a3df91 libQt5Test.so.5`QTest::TestMethods::invokeTestOnData(this=0x00007fffffffdf80, index=<unavailable>) const at qtestcase.cpp:941                                                                                                                                                                                                                                       
    frame #46: 0x00007ffff7a3edf3 libQt5Test.so.5`QTest::TestMethods::invokeTest(this=0x00007fffffffdf80, index=<unavailable>, data=0x0000000000000000, watchDog=0x0000000000000000) const at qtestcase.cpp:1140:37                                                                                                                                                                                   
    frame #47: 0x00007ffff7a3f557 libQt5Test.so.5`QTest::TestMethods::invokeTests(this=<unavailable>, testObject=0x00007fffffffe040) const at qtestcase.cpp:1484:43                                
    frame #48: 0x00007ffff7a3fbec libQt5Test.so.5`QTest::qRun() at qtestcase.cpp:1922:25                                                                                                           
    frame #49: 0x00007ffff7a3fd4c libQt5Test.so.5`QTest::qExec(testObject=<unavailable>, argc=<unavailable>, argv=<unavailable>) at qtestcase.cpp:1811:19                                          
    frame #50: 0x00005555555781eb testInternalWindow_waylandonly`main(argc=1, argv=0x00007fffffffe1c8) at internal_window.cpp:823:1                                                                
    frame #51: 0x00007ffff3812ee3 libc.so.6`__libc_start_main + 243                                                                                                                                
    frame #52: 0x0000555555568cee testInternalWindow_waylandonly`_start + 46
autotests/integration/internal_window.cpp
227

Change it to QTRY_COMPARE, so we will wait if no events have been dispatched. That's not accurate, but at least something...

Please leave a comment because this part is very weird. :/

davidedmundson added a subscriber: davidedmundson.EditedJun 27 2019, 1:39 PM

One nice pattern when it's not important whether something is processed an event loop or might have been done directly or via a previous event loop is:

QVERIFY(clientAddedSpy.count() ==1 || clientAddedSpy.wait());

we do that in a few places.

romangg added a comment.Jun 27 2019, 4:15 PM
In D22119#487289, @davidedmundson wrote:

One nice pattern when it's not important whether something is processed an event loop or might have been done directly or via a previous event loop is:

QVERIFY(clientAddedSpy.count() ==1 || clientAddedSpy.wait());

we do that in a few places.

That's definitely a nice pattern and if we already use it at other places I will change the diff to that. Only downside I could see is that it does not explicitly check that the signal has been sent only once when waiting. But that's not necessary here.

zzag added a comment.Jun 27 2019, 4:17 PM
In D22119#487347, @romangg wrote:

That's definitely a nice pattern and if we already use it at other places I will change the diff to that. Only downside I could see is that it does not explicitly check that the signal has been sent only once when waiting. But that's not necessary here.

You can do that with QTRY_COMPARE.

romangg added a comment.Jun 27 2019, 4:22 PM
In D22119#487348, @zzag wrote:
In D22119#487347, @romangg wrote:

That's definitely a nice pattern and if we already use it at other places I will change the diff to that. Only downside I could see is that it does not explicitly check that the signal has been sent only once when waiting. But that's not necessary here.

You can do that with QTRY_COMPARE.

True, I was under the impression it does something else. When I will use this one instead. Thanks!

romangg updated this revision to Diff 60735.Jun 27 2019, 4:23 PM
  • Use QTRY_COMPARE macro
romangg retitled this revision from [RFC] [autotests] In internal window test remove spy waits or guard them to [autotests] In internal window test remove spy waits or guard them.Jun 27 2019, 4:25 PM
zzag accepted this revision.Jun 27 2019, 4:37 PM
This revision is now accepted and ready to land.Jun 27 2019, 4:37 PM
Closed by commit R108:349560a78c47: [autotests] In internal window test remove spy waits or guard them (authored by romangg). · Explain WhyJun 27 2019, 4:38 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 60738

View Options

autotests/integration/internal_window.cpp

Loading...