Mailing-List: contact dev-help@harmony.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@harmony.apache.org
Received-SPF: pass (athena.apache.org: domain of gshimansky@gmail.com
 designates 64.233.178.242 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=message-id:date:from:user-agent:mime-version:to:subject:references:in-reply-to:content-type:content-transfer-encoding:sender;
        b=UVeoRUuzoWW1mS9OFAP2p5mZmUyCl+60Vq4z4s5d3twlyFKYO96CIG9cAxUm5/OfvI/tHWSP8WU3OQL1N65IgH2koCDea9EyRswIzuaz3eDT0fPDeNd1nT2q5jc7YSh1h46zXOr+264YR+5Mbmqp/ndhHMUWDOKNCiwnZuKG27w=
Message-ID: <475F2804.50704@apache.org>
Date: Wed, 12 Dec 2007 03:15:00 +0300
From: Gregory Shimansky <gshimansky@apache.org>
User-Agent: Thunderbird 2.0.0.9 (Windows/20071031)
MIME-Version: 1.0
To: dev@harmony.apache.org
Subject: Re: Eclipse bug was: [classlib][luni] TreeMap woes
References: <253b20230712110807h39c89a2awc04b2620289e5d0d@mail.gmail.com>
 <c3755b3a0712110947j28784f3cn7cc4e21184f1fc3@mail.gmail.com>
In-Reply-To: <c3755b3a0712110947j28784f3cn7cc4e21184f1fc3@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Sender: Gregory Shimansky <gshimansky@gmail.com>

Alexey Petrenko said the following on 11.12.2007 20:47:
> Sergey,
> 
> thanks for such detailed investigation.
> 
> It looks like we can keep the patch in M4.
> 
> Objections?

This is an impressive evaluation and it is very good that a bug was 
created in Eclipse bugzilla. If the impact of the new TreeMap is only 
several failing tests in EUT test suite then I see no reason to revert 
the patch, failing tests may be just excluded until Eclipse fixes this bug.

> 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.
>>

-- 
Gregory