Community
Participate
Working Groups
I recently added a test in Test_*_Browser: test_execute_and_closeListener(). (in Bug 509615 - [Webkit2] Test_BrowserSuite fails/hangs with tests.) This test executes javascript: "window.close()" and waits for CloseWindowListener to trigger. On Linux/Win32 this works. But on OSX it seems that the browser is closed but the listener is not triggered. As a result, the test fails on OS X. Could someone with SWT/OS X knowledge investigate why this functionality fails on OS X?
Details: - Look at the code of test_execute_and_closeListener - Trace is: > expected:<true> but was:<false> > java.lang.AssertionError: expected:<true> but was:<false> > at org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Widget.tearDown(Test_org_eclipse_swt_widgets_Widget.java:55) > at org.eclipse.test.EclipseTestRunner.run(EclipseTestRunner.java:749) > ... - This means that in Test_*_Widget, the assertion assertEquals(disposedIntentionally) is triggered. I.e Browser was closed but in the test the condition that sets disposedIntentionally=true was not met. Thus this test fails. Original build log: http://download.eclipse.org/eclipse/downloads/drops4/I20170206-2000/testresults/html/org.eclipse.swt.tests_ep47I-unit-win32_win32.win32.x86_8.0.html (these will be deleted in ~15 days or so).
Created attachment 266717 [details] snippet_to_reproduce_issue Attaching snippet. On Linux this prints "PASSED : Close listener was triggerd". I expect this not to print on OS X. Instead the browser will close without launching this handler. (Let me know if it does thou).
New Gerrit change created: https://git.eclipse.org/r/90598
Gerrit change https://git.eclipse.org/r/90598 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=27af2ad3ec898d8d1b5fb5a8f7716de046b66f6f
(In reply to Eclipse Genie from comment #3) > New Gerrit change created: https://git.eclipse.org/r/90598 Above was not a fix, just a note in the test linking to this bug.
I haven't seen this test fail on the Mac, but it does fail on Windows, also when run locally. The test in master doesn't make much sense, since it executes the window.close() Javascript *before* the SWT call to shell.open();. Bug 497962 made it again harder to track code history, but AFAICS, the implementation in IE.java:997 looks OK. But your hack in Test_org_eclipse_swt_browser_Browser.setUp():126 seems to be responsible for the platform difference that ultimately makes widget.isDisposed() in #tearDown() run on an unexpected widget: * See: Bug 499387 */ if (! SwtTestUtil.isWindows) { setWidget(browser); }
New Gerrit change created: https://git.eclipse.org/r/90759
(In reply to Eclipse Genie from comment #7) > New Gerrit change created: https://git.eclipse.org/r/90759 Hello Markus, Thank you for taking the time to investigate. (In reply to Markus Keller from comment #6) > I haven't seen this test fail on the Mac, but it does fail on Windows, also > when run locally. Strange, according to eclipse build logs it passed on Windows. But recent build logs also show it to pass on OSX. Maybe it fails sporadically on those platforms. > The test in master doesn't make much sense, since it executes the > window.close() Javascript *before* the SWT call to shell.open();. That's a good point. (On Linux it triggers the listener even when the shell is not open, but I see where you're coming from). I've moved the browser.execute("window.close()") into a progress completion listener. I've submitted a preview patch: https://git.eclipse.org/r/90759 Is it possible for you to check it out and see if this makes the test pass? If so, I'll submit a proper patch. > Bug 497962 made it again harder to track code history, but AFAICS, the > implementation in IE.java:997 looks OK. If test still fails, can you do me a favour and run the attached snippet_to_reproduce and let me know if it prints anything to the console? This should help me understand how windows does it's thing. > But your hack in Test_org_eclipse_swt_browser_Browser.setUp():126 This method is used in many test setups. See: Test_*_(CCombo|Canvas|Combo|Coolbar|Group|Spinner|Tree etc..) It just behaves differently on windows than on OSX/Linux. Does the test or snippet behave differently without it? I don't think that's the cause thou, because in test_execute_and_close, if the close listener gets triggered, then disposedIntentionally is set to true and setWidget(browser) should not make a difference? Thank you for your time.
I should setup a win32/cocoa test box to investigate why the test fails sporadically on those platforms.
New Gerrit change created: https://git.eclipse.org/r/91325
Gerrit change https://git.eclipse.org/r/91325 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=1133c09eab77d11fb4416493fef47559497f59a8
I temporarily dissabled the test on Win32/Cocoa. I'm going on vacation for 2 weeks, after I'll setup a win32/Cocoa box to investigate this further.
(In reply to Leo Ufimtsev from comment #8) This is not a sporadic failure. test_execute_and_closeListener() never failed on macOS, but it consistently failed on Windows. test_BrowserFunction_callback_with_multipleValues fails on macOS, but that's unrelated to this bug. > > But your hack in Test_org_eclipse_swt_browser_Browser.setUp():126 > > This method is used in many test setups. See: The hack is that the Test*Browser#setUp() method no longer calls setWidget(browser) on Windows. The comment there already tells that the widget hierarchy is not the same on all platforms. You have to debug this on Windows.
(In reply to Markus Keller from comment #13) > (In reply to Leo Ufimtsev from comment #8) > This is not a sporadic failure. test_execute_and_closeListener() never > failed on macOS, but it consistently failed on Windows. > > test_BrowserFunction_callback_with_multipleValues fails on macOS, but that's > unrelated to this bug. > > > > But your hack in Test_org_eclipse_swt_browser_Browser.setUp():126 > > > > This method is used in many test setups. See: > > The hack is that the Test*Browser#setUp() method no longer calls > setWidget(browser) on Windows. The comment there already tells that the > widget hierarchy is not the same on all platforms. You have to debug this on > Windows. I've spent some time and setup a win32 box and got SWT development going (except rebuilding bindings). After debugging a bit, I see what you mean when you say the breakage is due to the 'hack' (setwidget). This is an error on my part, thanks for looking into this. I'll look for a solution tomorrow.
New Gerrit change created: https://git.eclipse.org/r/92555
Test_*_Browser#test_execute_and_closeListener() was actually working if setWidget() is called in the constructor. But calling setWidget() in the constructor would cause test_getChildren() to fail. The reason is that Browser on Win32 has 1 child (an OleFrame). See: https://bugs.eclipse.org/bugs/show_bug.cgi?id=499387#c22 The solution in this case is to remove the "if (!windows) {" condition in the constructor and instead override test_getChildren in Browser and treat it differently on Win32. @Niraj / Markus. If you have time, could you take a quick peak at the provided patch and let me know if you have any concerns. If not, I'll merge it later.
Gerrit change https://git.eclipse.org/r/92555 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=c5e549481a618579dda9423425a8862a9672e314
I pushed the patch to master. (If there is feedaback about the patch, feel free to post it here). There shouldn't be any SWT Browser jUnits that still fail now. I'll double check build logs tomorrow to verify.
No errors in build logs. All tests passed.
