Bugzilla – Full Text Bug Listing
| Summary: | Virtual tree test failures on Mac OS 10.6.7 | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Oleg Besedin <ob1.eclipse> | ||||||||||||
| Component: | UI | Assignee: | Platform-UI-Inbox <Platform-UI-Inbox> | ||||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||||
| Severity: | normal | ||||||||||||||
| Priority: | P3 | CC: | bokowski, daniel_megert, eclipse.felipe, markus.kell.r, Mike_Wilson, ob1.eclipse, Olivier_Thomann, prakash, pwebster, remy.suen, Silenio_Quarti | ||||||||||||
| Version: | 3.7 | Flags: | bokowski:
review+
pwebster: review+ remy.suen: review+ markus.kell.r: review+ |
||||||||||||
| Target Milestone: | 3.7 RC4 | ||||||||||||||
| Hardware: | PC | ||||||||||||||
| OS: | Mac OS X | ||||||||||||||
| Whiteboard: | |||||||||||||||
| Attachments: |
|
||||||||||||||
See http://dev.eclipse.org/mhonarc/lists/platform-releng-dev/msg18589.html . Also, the list of failing tests looks similar to the tests that fail on Windows when something interferes with the display. We did not have those test fail on Mac prior to the recent OS upgrade. As far as I know, there there no recent code changes in the JFace that could be attributed to those failed tests. Created attachment 196799 [details]
stack trace in 10.5
Silenio was able to duplicate test failures. It appears that OS messages responsible for drawing and things like #setData() come later on Mac 10.6 when they used to. For instance, for the SimpleVirtualLazyTreeViewerTest#testCreation(), depending on the luck of a run, the tree might not get drawn until we get to the #tearDown(). Adding a liberal amount of display loops "fixes" the test. Similarly, for SimpleVirtualLazyTreeViewerTest#testRemoveAt() the widget's setData() method don't seem to be processed by the time we try to set selection. Again, sprinkling display loops through the code "fixes" the test. Created attachment 196862 [details]
Modified SWT snippet202
Here is a slightly modified SWT's snippet 202 that shows the problem without JFace layer.
On Windows both checks pass.
On Mac 10.6.7 test for "tree.getItemCount()" passes, but updateElementCallCount tets fails. On Mac events are received in the second call to display loop.
(In reply to comment #4) > Here is a slightly modified SWT's snippet 202 that shows the problem without > JFace layer. Moving back to SWT for investigation. (Silenio wont be back till next week, so I am trying to figure out what is going on in the m). For me, the simple answer is that readAndDispatch() is returning false when it should be returning true. Internally to SWT, application.nextEventMatchingMask() is returning null (which indicates that the event queue is empty, thus the application can go to sleep). But, application.nextEventMatchingMask() is causing work to happen, selectors are running, even SWT paint events get dispatched. In this case, the first call to readAndDispatch() (which IMO is wrongly returning false), is in fact causing several selectors to run, including: sel_setNeedsDisplay_ and sel_setNeedsDisplayInRect_ on the scrollbars, and sel_updateTrackingAreas on the tree and shell. Oleg, is there a case (in the product) where the application actually needs all calls to setdata to happen before a certain time ? (In reply to comment #6) > Oleg, is there a case (in the product) where the application actually needs all > calls to setdata to happen before a certain time ? Boris tested several potentially problematic areas (such as Debug call stack, Ctrl+3, search view) and they seem to be working fine. So far we (Platform/UI) are not aware of any actual problems in the SDK surfaced by this issue. Created attachment 196926 [details]
(hack) Patch
Oleg, could you please try running the tests with this patch.
Basically as long as there are selectors being invoked readAndDispatch() will not return false.
I do not recommend this fix for 3.7 at this point in the release.
If the code is wrong, the application can get "stuck" in a case where readAndDispatch() return false forever and never goes to sleep.
I also haven't seen any issues in the SDK during the test passes. I agree this should not be touched for 3.7. (In reply to comment #8) > Created attachment 196926 [details] > (hack) Patch For me, it only fixes one of the failing tests (AbstractTreeViewerTest#testRefreshWithAddedChildren). Created attachment 197021 [details]
Patch - temporary disabling affected tests on Mac
Boris, Paul, Remy, could you review "Patch - temporary disabling affected tests on Mac" patch for RC4? The patch temporarily disables failing tests on Mac Cocoa. The patch only changes test code and has no affect on the SDK code we ship. The patch is not a fix for the underlying event queue problem. Looks straight forward. PW Could it be that your patch disables too many test cases? Some of these test classes are being used for non-virtual trees as well. (In reply to comment #14) > Could it be that your patch disables too many test cases? Some of these test > classes are being used for non-virtual trees as well. The StructuredViewerTest? I wanted the change to be as simple as possible for RC4. (In reply to comment #15) > (In reply to comment #14) > > Could it be that your patch disables too many test cases? Some of these test > > classes are being used for non-virtual trees as well. > > The StructuredViewerTest? I wanted the change to be as simple as possible for > RC4. I put a conditional breakpoint in org.eclipse.jface.tests.viewers.AbstractTreeViewerTest.testRefreshWithAddedChildren() that printed something, and got the following output in the console: Breakpoint hit! Breakpoint hit! Breakpoint hit! Breakpoint hit! Breakpoint hit! Breakpoint hit! Since we didn't see six failures from testRefreshWithAddedChildren alone, your patch disables too many test cases. One possible way to only disable the tests that we need to disable is doing the following: if (disableTestsBug347491 && (fViewer.getControl().getStyle() & SWT.VIRTUAL) != 0) return; Created attachment 197043 [details]
Patch - temporary disabling affected tests Update 1
Here is the updated patch. Boris, could you try it on your Mac?
+1 - the tests pass with the new patch, and it will only disable the failing tests. (forgot to also set the review + flag) My +1 got removed accidentally. PW (In reply to comment #17) > Created attachment 197043 [details] > Patch - temporary disabling affected tests Update 1 Breakpoints are only hit where applicable and all affected files have had their copyrights updated, looks good to me, +1. (In reply to comment #17) > Created attachment 197043 [details] [diff] > Patch - temporary disabling affected tests Update 1 +1 for RC4. Verified on Cocoa with 10.6 and on Windows 7. Patch applied to CVS Head. Thank you! I cloned this bug to bug 347947 for SWT fix to the event loop and bug 347950 to remind us to restore tests in 3.8 as the underlying issue gets fixed. I'll rename this bug to reflect that it deals with disabling tests only; please continue discussion of the fix in the bug 347947. Verified that there were no UI test failures in build I20110602-1051 and I20110602-0100. |
We have several tests in JFace test suite that started intermittently fail after the recent Mac OS upgrade. The tests seems to fail on 10.6.7 using both 64 bit and 32 bit VM. The tests fail for both 3.6.2 and 3.7. The tests pass on 10.5.8. This means that there is likely change in behavior in Mac OS 10.6. The failing tests: SimpleVirtualLazyTreeViewerTest testCreation() assertTrue("call to updateElement expected", updateElementCallCount > 0); testRemoveAt() assertEquals(2, ((IStructuredSelection) treeViewer.getSelection()) .size()); AbstractTreeViewerTest testRefreshWithAddedChildren() assertNotNull("new child is visible", fViewer.testFindItem(child)); StructuredViewerTest testDeleteSibling() assertNotNull("first child is visible", fViewer.testFindItem(first)); testInsertSibling() assertNotNull("new sibling is visible", fViewer .testFindItem(newElement)); testInsertSiblings() assertNotNull("new siblings are visible", fViewer .testFindItem(newElements[i])); testSetInput() assertNotNull("first child is visible", fViewer .testFindItem(firstfirst)); testSomeChildrenChanged() assertNotNull("new sibling is visible", fViewer .testFindItem(newElement)); testWorldChanged() assertNotNull("new sibling is visible", fViewer .testFindItem(newElement)); http://download.eclipse.org/eclipse/downloads/drops/I20110526-1708/testresults/xml/org.eclipse.ui.tests_macosx.cocoa.x86_5.0.xml