Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 313313 - [implementation][preferences][spell checking] Move disabling of spelling for performance tests into tests itself
Summary: [implementation][preferences][spell checking] Move disabling of spelling for ...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 3.3   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.7 M3   Edit
Assignee: Deepak Azad CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-18 05:59 EDT by Dani Megert CLA
Modified: 2010-10-26 06:35 EDT (History)
1 user (show)

See Also:
daniel_megert: review+

Attachments
fix (11.51 KB, patch)
2010-10-05 22:07 EDT, Deepak Azad CLA 		daniel_megert: review-
Details | Diff
fix (6.56 KB, text/plain)
2010-10-06 10:38 EDT, Deepak Azad CLA 		no flags Details
Show Obsolete (1) 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 Dani Megert CLA 2010-05-18 05:59:11 EDT 
R3.3.

We should move the code that disables spelling for performance tests from AbstractDecoratedTextEditorPreferenceConstants into tests itself.

Currently, when running the performance tests on a local machine we get different results than on the official/releng test machines.
Comment 1 Deepak Azad CLA 2010-10-05 22:07:45 EDT 
Created attachment 180291 [details]
fix

The patch 
- removes the spell check disabling code from AbstractDecoratedTextEditorPreferenceConstants
- disables spell checking for all tests in o.e.jdt.text.tests.performance package
Comment 2 Deepak Azad CLA 2010-10-05 22:17:55 EDT 
Frederic, this change may affect some other performance tests numbers if they deal with text editors.
Comment 3 Dani Megert CLA 2010-10-06 05:03:57 EDT 
- DisableAutoBuildTestSetup now has side-effects
- If a single test is executed then spelling would still be enabled :-(

==> Why don't you simply add it in TextPerformanceTestCase.setUp/tearDown? AFAIK most if not all text performance test cases inherit from that class.
Comment 4 Deepak Azad CLA 2010-10-06 10:38:10 EDT 
Created attachment 180336 [details]
fix

(In reply to comment #3)
> ==> Why don't you simply add it in TextPerformanceTestCase.setUp/tearDown?
> AFAIK most if not all text performance test cases inherit from that class.
I tried to enable/disable the spell check once per all tests, but as discussed I now add that to TextPerformanceTestCase.setUp/tearDown.
Comment 5 Dani Megert CLA 2010-10-11 11:34:50 EDT 
Patch looks good with two minor changes that I made before committing to HEAD:
- TextPerformanceTestCase2: moved super.tearDown() to the end
- OpenPreferencePageTest added super calls to setUp() and tearDown()

Filed bug 327459 to verify the next performance results.
Comment 6 Dani Megert CLA 2010-10-26 06:35:41 EDT 
Verified in I20101025-1800.