Bug 231863 - ScrollVerticalRuler5000Test should be dark green
Summary: ScrollVerticalRuler5000Test should be dark green
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 3.4   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.4 RC1   Edit
Assignee: Benno Baumgartner CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2008-05-13 13:01 EDT by Benno Baumgartner CLA
Modified: 2008-05-15 06:32 EDT (History)
1 user (show)

See Also:
daniel_megert: review+
daniel_megert: review+

Attachments
let test fail if not enough annotations in editor (3.99 KB, patch)
2008-05-13 13:01 EDT, Benno Baumgartner CLA 		no flags Details | Diff
fix HEAD (2.37 KB, patch)
2008-05-15 04:05 EDT, Benno Baumgartner CLA 		no flags Details | Diff
fix perf_33x (16.61 KB, patch)
2008-05-15 04:16 EDT, Benno Baumgartner CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Benno Baumgartner CLA 2008-05-13 13:01:39 EDT 
Created attachment 99987 [details]
let test fail if not enough annotations in editor

I20080510-2000

The ScrollVerticalRuler5000Test should show a speed up of around 3 after fix for bug 113247 has been released for I20080320-0800. I can reproduce that on my machine and on our build machine. But the ottawa test machine shows no effect at all:
http://download.eclipse.org/eclipse/downloads/drops/I20080508-2000/performance/eclipseperflnx2/org.eclipse.jdt.text.tests.performance.ScrollVerticalRuler5000Test.testScrollTextEditorPageWise().html

The only explanation I can offer is that the test is bogus. I assume, that the editor is scrolled on the test machine without any annotations at all. This maybe due to a missing spelling dictionary or another bug in the spelling engine. 

I want to release the attached patch which asserts that there are enough annotations in the editor to prove my theory.

Dani? Any better ideas?
Comment 1 Dani Megert CLA 2008-05-14 08:36:45 EDT 
Please commit the change and mark your calender to check the outcome.
Comment 2 Benno Baumgartner CLA 2008-05-14 09:34:06 EDT 
first patch released > I20080513-2000
Comment 3 Dani Megert CLA 2008-05-15 02:24:48 EDT 
OK, got it: your test relies on spelling being enabled, however, this is off by default when running the tests on the build machine.

1. remove assertion again
2. enable (and then disable) spelling for all your tests where you rely on it
3. backport to 3.3.x perf branch
Comment 4 Benno Baumgartner CLA 2008-05-15 04:05:02 EDT 
Created attachment 100377 [details]
fix HEAD

I see, there's code in AbstractDecoratedTextEditorPreferenceConstants which turns off the spelling engine if the db location system property is set, great:-)

I'll keep the assertion, just to make sure. I've even added another one which checks that a spelling engine is installed.
Comment 5 Benno Baumgartner CLA 2008-05-15 04:16:14 EDT 
Created attachment 100378 [details]
fix perf_33x

Dani, if I release both patches now then the tests will go dark red until a new baseline is created (which may take weeks...) If I don't release the HEAD fix then the assertion will fail. I would like to release both now...
Comment 6 Dani Megert CLA 2008-05-15 05:40:19 EDT 
Please go ahead but make sure to *put* the value and not just set the default and make sure to call setToDefault in tearDown.
Comment 7 Benno Baumgartner CLA 2008-05-15 06:32:27 EDT 
fixed > I20080513-2000 and also in perf_33x stream