harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexey Petrenko" <alexey.a.petre...@gmail.com>
Subject Re: Eclipse bug was: [classlib][luni] TreeMap woes
Date Tue, 11 Dec 2007 17:47:50 GMT
Sergey,

thanks for such detailed investigation.

It looks like we can keep the patch in M4.

Objections?

SY, Alexey

2007/12/11, Sergey Kuksenko <sergey.kuksenko@gmail.com>:
> Hi All,
> Here is a story why we get EUT failures after new TreeMap.
>
>
> After new TreeMap implementation we got 3 error in EUT: testAddWorkingSet,
> testGetWorkingSet and testRemoveWorkingSet.
>
> The problem here that when this unit tests are running we got incorrect set
> of WorkingSets obtained from WorkingSetManager.
> Another problem - if we run these tests separately all of them are passed.
>
> I did mixed TreeMap implementation: aggregates old and new TreeMaps and
> compare trees after each operation.
> When did it I got error (different trees) in test
> org.eclipse.core.resources.IWorkspace.IWorkingSetManagerTest.
>
> I have to note that there are a lot of other unit tests executed between
> IWorkingSetManagerTest and <>.
> The tree difference error occurs for all test from IWorkingSetManagerTest
> class, because of the error was in "doSetUp()" method.
> Here is a part of code from "doSetUp()":
>
>   IWorkingSet[] workingSets = fWorkingSetManager.getWorkingSets();
>   for (int i = 0; i < workingSets.length; i++) {
>       fWorkingSetManager.removeWorkingSet(workingSets[i]);
>   }
>
> WorkingSetManager store set of WorkingSets in SortedSet (TreeSet).
>
> getWorkingSets() returs some filtered array of WorkingSets. In this unit
> test the first WorkingSet is filtered (as global WS),
> thus after this cycle we should get set of WorkingSets which contains only
> one WS, but it did not occur.
> Otherwords, after this cycle getWorkingSets() should return empty array (all
> non-filtered WS should be removed).
>
> TreeSet contains object with "org.eclipse.ui.internal.WorkingSet" class and
> uses
> "org.eclipse.ui.internal.WorkingSetComparator".
> This classes are part of "org.eclipse.ui.workbench" plugin.
>
> "WorkingSetComparator" compares two "WorkingSet"s using their labels and
> names as strings. Here, before removing, TreeSet contains
> 3 WS objects:
> { "Window Working Set", " ", "ws1" } - this is TreeSet in order which
> elements are stored, I put here labels of WorkingSets, because of it is
> sufficient for understading behavior of comparator.
>
> Here(before removing) we have such ThreeSet for old TreeSet, new TreeSet and
> even on RI!
>
> But! It is already incorrect TreeSet! Because of " " is less then "Window
> Working Set".
> The only difference between old TreeSet/RI and new TreeSet that new TreeSet
> doesn't delete WS with name " " succesfully.
> And as I alredy wrote succesfull removing WS with name " " at this place on
> RI was accidental.
>
> Moreover, I found original source of the problem.
> Class org.eclipse.ui.tests.api.IWorkingSetTest is excecuted before
> IWorkingSetManagerTest.
> IWorkingSetTest contains unit tests "testSetName()" which tests that chaning
> name of WorkingSet works succesfully.
>
> public void testSetName() throws Throwable {
>         boolean exceptionThrown = false;
>         try {
>             fWorkingSet.setName(null);
>         } catch (RuntimeException exception) {
>             exceptionThrown = true;
>         }
>         assertTrue(exceptionThrown);
>
>         fWorkingSet.setName(WORKING_SET_NAME_2);
>         assertEquals(WORKING_SET_NAME_2, fWorkingSet.getName());
>
>         fWorkingSet.setName("");
>         assertEquals("", fWorkingSet.getName());
>
>         fWorkingSet.setName(" ");
>         assertEquals(" ", fWorkingSet.getName());
> }
>
> And after "testSetName()" TreeSet stored in workingSetManager are cleaned in
> order to have empty TreeSet for all subsequent unit tests.
> There are call "workingSetManager.removeWorkingSet(fWorkingSet)" where
> fWorkingSet is WS with name " ". But because
> of originally it was saved in Tree with the inital name ("ws1") then remving
> are not performed even on RI.
>
> So here we meet a bug in Eclipse and sometimes the bug leads to incorrect
> TreeSet on RI and with the bug Eclipse is working on RI.
> So I suppose that here having incorrect TreeSet is not critical for Eclipse
> functionality.
>
>
> Let's add aditional checks into "testSetName()" unit test.
>
>  assertTrue(ArrayUtil.equals(new IWorkingSet[] { fWorkingSet },
> workingSetManager.getWorkingSets()));
>  workingSetManager.removeWorkingSet(fWorkingSet);
>  assertTrue(ArrayUtil.equals(new IWorkingSet[] {},
> workingSetManager.getWorkingSets()));
>  workingSetManager.addWorkingSet(fWorkingSet);
>
> The first assert here checks that fWorkingSet belongs to the TreeSet from
> setManager, after that we remove the working set,
> the second assert checks that working set was succesfully removed, and the
> latest line restore workingSetManager state.
>
> I've inserted these line at the beginning of "testSetName()" and at the end.
> The first insertion checks that we can succesfully remove WS before
> renaming, and the latest checks that we should be able remove WS after
> remaning.
>
> I've got error on the last assert which proves that after remaning WS can't
> be removed on Sun 1.6u3 and on BEA JRockit1.5
>
> Bug 212583 was submitted against Eclipse.
>

Mime
View raw message