Summary: | ITypeHierarchyChangedListener receive spurious callbacks | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Jason Parekh <jasonparekh> | ||||||||||
Component: | Core | Assignee: | Jay Arthanareeswaran <jarthana> | ||||||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||||||
Severity: | normal | ||||||||||||
Priority: | P3 | CC: | Olivier_Thomann, satyam.kandula, srikanth_sankaran, tparker | ||||||||||
Version: | 3.7 | Flags: | srikanth_sankaran:
review+
|
||||||||||
Target Milestone: | 3.7 M1 | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Linux | ||||||||||||
Whiteboard: | |||||||||||||
Attachments: |
|
Description
Jason Parekh
2010-06-11 16:27:52 EDT
I am seeing the problem. I will investigate. Created attachment 171912 [details]
Proposed patch
Proposed patch + tests. More tests might be added.
Created attachment 172902 [details]
Updated patch
Added a new tests in TypeHierarchyNotificationTests.testAddNonJavaToPackageFragmentRoot_b(). However this tests is failing because the code that processes the fine grained changes seems to be broken. Will investigate further and raise a new bug if required.
Created attachment 173286 [details]
Patch with updated tests
The failing test has been fixed. All tests pass.
Srikanth, would you have some time to review this patch? Thanks! (In reply to comment #5) > Srikanth, would you have some time to review this patch? Thanks! I think the patch is on the right track, however (1) I don't think the tests are testing the right thing: You can back out the source change and run the new tests and 2/3 pass. This indicates that the tests are not precise enough. (2) Should org.eclipse.jdt.core.tests.model.TypeHierarchyNotificationTests.testBug316654() cleanup the txt file that is being created ? (3) For org.eclipse.jdt.core.tests.model.TypeHierarchyNotificationTests.testBug316654_b() the javadoc says it can throw CoreException, but the method throw spec has Exception in it -- Is this intentional ? Created attachment 173395 [details]
Updated pat
Patch incorporates comments (2) and (3).
About (1) - Those are regression tests that should pass with and without the fix. Since I didn't find any existing tests for those scenarios, I added them anyway. Test TypeHierarchyNotification#testBug316654_b() is particularly important because it is directly related to the code that is being changed in this patch.
Added additional test in TypeHierarchyNotificationTests.testBug316654_c(), which is similar to testBug316654_b() except that it tests a change to an internal JAR. Released in HEAD for 3.7M1. Verified for 3.7M1 using build I20100802-1800 |