Bugzilla – Full Text Bug Listing
|
Description
Leo Ufimtsev
New Gerrit change created: https://git.eclipse.org/r/78650 Gerrit change https://git.eclipse.org/r/78650 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=87f893f36b719c566a01e02580f73c322405087f (In reply to Eclipse Genie from comment #2) > Gerrit change https://git.eclipse.org/r/78650 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=87f893f36b719c566a01e02580f73c322405087f In master, leaving bug open as per Leo's request. New Gerrit change created: https://git.eclipse.org/r/78707 Gerrit change https://git.eclipse.org/r/78707 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=21a37adf8fd9364135a61c4078af874351ef4735 (In reply to Eclipse Genie from comment #5) > Gerrit change https://git.eclipse.org/r/78707 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=21a37adf8fd9364135a61c4078af874351ef4735 In master now. New Gerrit change created: https://git.eclipse.org/r/78712 Gerrit change https://git.eclipse.org/r/78712 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=a5c5d6ba31bbcbb84ac01132ebfbc4638cde645f (In reply to Eclipse Genie from comment #8) > Gerrit change https://git.eclipse.org/r/78712 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=a5c5d6ba31bbcbb84ac01132ebfbc4638cde645f In master now. New Gerrit change created: https://git.eclipse.org/r/78714 Gerrit change https://git.eclipse.org/r/78714 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=e3f5a35f669c9d9fdf0f793dc77008442d4c788e the 10 second tests now run in 2-3 seconds. ~improvement. I'll check the build logs tomorrow to confirm that updates didn't break build servers. I checked build logs today, browser test 7,8,9 passed fine after modifications. Tests now run ~2x faster. I can't see a straight forward way to make other tests faster without sacrificing stability. Some work left to be done in Test_*_Browser.. Will be submitting a few patches. Tests would also benifit from a bit of documentation and proper naming. (Browser1, Brower2 is not very readable). New Gerrit change created: https://git.eclipse.org/r/82474 Gerrit change https://git.eclipse.org/r/82474 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=07e80ca640ed8f7d2ee3d46196f0fc522d9d8b55 New Gerrit change created: https://git.eclipse.org/r/82478 Gerrit change https://git.eclipse.org/r/82478 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=120dd7b3ac54a01cc73a92f5e43ded12c010a0d0 Recent changes made as part of this bug has caused below test failure on Windows: http://download.eclipse.org/eclipse/downloads/drops4/I20161005-1430/testresults/html/org.eclipse.swt.tests_ep47I-unit-win32_win32.win32.x86_8.0.html#AllBrowserTests Can see this particular JUnit failing locally as well on Win7. (In reply to comment #19) > Recent changes made as part of this bug has caused below test failure on > Windows: > http://download.eclipse.org/eclipse/downloads/drops4/I20161005-1430/testresults/html/org.eclipse.swt.tests_ep47I-unit-win32_win32.win32.x86_8.0.html#AllBrowserTests > > Can see this particular JUnit failing locally as well on Win7. Sorry for trouble. Let me investigate. It might be because I switched the parent of browser. (In reply to Niraj Modi from comment #19) > Recent changes made as part of this bug has caused below test failure on > Windows: > http://download.eclipse.org/eclipse/downloads/drops4/I20161005-1430/ > testresults/html/org.eclipse.swt.tests_ep47I-unit-win32_win32.win32.x86_8.0. > html#AllBrowserTests > > Can see this particular JUnit failing locally as well on Win7. I think the following could have broken it on windows is: org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser.setUp() { + setWidget(browser); } I lack a Windows test setup. Can you please try to delete that line and see if the jUnit works after that? The reason I added that line was that otherwise the browser test cases share the shell with an empty composite. It's also what other test classes do, ex: org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_ToolBar.setUp() Let me know. If that fixes the tests, I'll revert that line for now and try to look for another solution. Thank you for your time. (In reply to Leo Ufimtsev from comment #21) > (In reply to Niraj Modi from comment #19) > > Recent changes made as part of this bug has caused below test failure on > > Windows: > > http://download.eclipse.org/eclipse/downloads/drops4/I20161005-1430/ > > testresults/html/org.eclipse.swt.tests_ep47I-unit-win32_win32.win32.x86_8.0. > > html#AllBrowserTests > > > > Can see this particular JUnit failing locally as well on Win7. > > I think the following could have broken it on windows is: > org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser.setUp() { > + setWidget(browser); > } > > I lack a Windows test setup. Can you please try to delete that line and see > if the jUnit works after that? > > The reason I added that line was that otherwise the browser test cases share > the shell with an empty composite. It's also what other test classes do, ex: > org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_ToolBar.setUp() > > Let me know. If that fixes the tests, I'll revert that line for now and try > to look for another solution. > > Thank you for your time. Yes, reverting the setWidget(browser) line fixes the problem. Reason for this issue: After debugging this realized that IE Browser doesn't have 0 children. Minimum child count for IE Browser will be 1, which is unlike reset of the composite classes. Please see: IE#create(Composite parent, int style) method, we add OleFrame instance while creating IE browser. New Gerrit change created: https://git.eclipse.org/r/82740 (In reply to Eclipse Genie from comment #23) > New Gerrit change created: https://git.eclipse.org/r/82740 @ niraj I submitted a patch that reverts the behaviour for windows. (But maintains for other platforms). Please let me know if this patch is satisfactory or if you have an idea for another patch. Thank you for your time. Gerrit change https://git.eclipse.org/r/82740 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=1b918f3a0bfb74017a88d3f0dc20e8fc438719c3 (In reply to Leo Ufimtsev from comment #24) > (In reply to Eclipse Genie from comment #23) > > New Gerrit change created: https://git.eclipse.org/r/82740 > > @ niraj > I submitted a patch that reverts the behaviour for windows. (But maintains > for other platforms). > Please let me know if this patch is satisfactory or if you have an idea for > another patch. > > Thank you for your time. Thanks Leo for the fix, have verified it on my Win7 setup, pushed to master. (In reply to comment #26) > Thanks Leo for the fix, have verified it on my Win7 setup, pushed to master. Nice. I added a test case to Browser. *Theoretically* should work across all platforms :-). See: 505591: Add test case for Browser.evaluate() to browser test suite. https://bugs.eclipse.org/bugs/show_bug.cgi?id=505591 Re-opening to tidy up some Listener-related tests. New Gerrit change created: https://git.eclipse.org/r/90451 Created attachment 266677 [details] Before/After listeners tidy up (In reply to Eclipse Genie from comment #29) > New Gerrit change created: https://git.eclipse.org/r/90451 I was working on listeners and found it time consuming to find all test cases related to a particular listener. So I tidied things up. New Gerrit change created: https://git.eclipse.org/r/90462 (In reply to Eclipse Genie from comment #31) > New Gerrit change created: https://git.eclipse.org/r/90462 Refactoring duplicated code into lambdas. I will break down the large commit into smaller once. To be continued in: Bug 511849 - [Webkit] Refactor Browser Listener tests |