Bug 442754 - Avoid using a static constant to give production code

different behavior during a test

Change-Id: I115677ee2a5b6b7ccceb75949678d31a08f01732
Signed-off-by: Nathan Ridge <zeratul976@hotmail.com>
Reviewed-on: https://git.eclipse.org/r/36885
Tested-by: Hudson CI
Reviewed-by: Sergey Prigogin <eclipse.sprigogin@gmail.com>
Tested-by: Sergey Prigogin <eclipse.sprigogin@gmail.com>

core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/SemanticHighlightingTest.java

@@ -15,16 +15,20 @@ package org.eclipse.cdt.ui.tests.text;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.runtime.NullProgressMonitor;
 import org.eclipse.core.runtime.Path;
 import org.eclipse.jface.preference.IPreferenceStore;
+import org.eclipse.jface.preference.PreferenceConverter;
 import org.eclipse.jface.text.BadPositionCategoryException;
 import org.eclipse.jface.text.IDocument;
 import org.eclipse.jface.text.Position;
 import org.eclipse.jface.text.source.SourceViewer;
+import org.eclipse.swt.graphics.RGB;
 
 import junit.framework.Test;
 import junit.framework.TestCase;
@@ -52,8 +56,8 @@ import org.eclipse.cdt.internal.ui.editor.CEditor;
 import org.eclipse.cdt.internal.ui.editor.SemanticHighlighting;
 import org.eclipse.cdt.internal.ui.editor.SemanticHighlightingManager;
 import org.eclipse.cdt.internal.ui.editor.SemanticHighlightingPresenter;
-import org.eclipse.cdt.internal.ui.editor.SemanticHighlightingPresenter.TestHighlightedPosition;
 import org.eclipse.cdt.internal.ui.editor.SemanticHighlightings;
+import org.eclipse.cdt.internal.ui.editor.SemanticHighlightingManager.HighlightedPosition;
 
 /**
  * Semantic highlighting tests.
@@ -74,6 +78,12 @@ public class SemanticHighlightingTest extends TestCase {
 	private IIndex fIndex;
 	private IASTTranslationUnit fAST;
 	
+	// The highlighted positions stored in the document don't store any information
+	// that directly identifies which highligting they are for. To recover this
+	// information, we assign a different color to each highlighting, and then
+	// look up the highlighting's preference key based on the color.
+	private Map<RGB, String> fColorToPreferenceKeyMap;
+	
 	private static File createExternalFile(final String code) throws Exception {
 		File dest = File.createTempFile("external", ".h");
 		FileOutputStream fos = new FileOutputStream(dest);
@@ -82,18 +92,28 @@ public class SemanticHighlightingTest extends TestCase {
 		return dest;
 	}
 
-	private static void enableAllSemanticHighlightings() {
+	private void enableHighlightingsAndAssignColors() {
+		fColorToPreferenceKeyMap = new HashMap<>();
 		IPreferenceStore store= CUIPlugin.getDefault().getPreferenceStore();
 		store.setValue(PreferenceConstants.EDITOR_SEMANTIC_HIGHLIGHTING_ENABLED, true);
-		SemanticHighlighting[] semanticHilightings= SemanticHighlightings.getSemanticHighlightings();
+		SemanticHighlighting[] semanticHighlightings= SemanticHighlightings.getSemanticHighlightings();
-		for (SemanticHighlighting semanticHilighting : semanticHilightings) {
+		int blue = 0;  // for assigning colors to preferences below
-			String enabledPreferenceKey = SemanticHighlightings.getEnabledPreferenceKey(semanticHilighting);
+		for (SemanticHighlighting semanticHighlighting : semanticHighlightings) {
+			// Enable the highlighting.
+			String enabledPreferenceKey = SemanticHighlightings.getEnabledPreferenceKey(semanticHighlighting);
 			if (!store.getBoolean(enabledPreferenceKey))
 				store.setValue(enabledPreferenceKey, true);
+			
+			// Choose a unique color for the highlighting, and save the mapping
+			// from the color to the highlighting's preference key .
+			String colorPreferenceKey = SemanticHighlightings.getColorPreferenceKey(semanticHighlighting);
+			RGB color = new RGB(0, 0, blue++);  // every highlighting gets a different shade of blue
+			PreferenceConverter.setValue(store, colorPreferenceKey, color);
+			fColorToPreferenceKeyMap.put(color, semanticHighlighting.getPreferenceKey());
 		}
 	}
 	
-	private static void restoreAllSemanticHighlightingToDefaults() {
+	private void restorePreferencesToDefaults() {
 		IPreferenceStore store= CUIPlugin.getDefault().getPreferenceStore();
 		store.setToDefault(PreferenceConstants.EDITOR_SEMANTIC_HIGHLIGHTING_ENABLED);
 		SemanticHighlighting[] semanticHighlightings= SemanticHighlightings.getSemanticHighlightings();
@@ -101,16 +121,17 @@ public class SemanticHighlightingTest extends TestCase {
 			String enabledPreferenceKey= SemanticHighlightings.getEnabledPreferenceKey(semanticHighlighting);
 			if (!store.isDefault(enabledPreferenceKey))
 				store.setToDefault(enabledPreferenceKey);
+			String colorPreferenceKey = SemanticHighlightings.getColorPreferenceKey(semanticHighlighting);
+			store.setToDefault(colorPreferenceKey);
 		}
+		fColorToPreferenceKeyMap.clear();
 	}
 	
 	@Override
 	protected void setUp() throws Exception {
 		super.setUp();
 		
-		enableAllSemanticHighlightings();
+		enableHighlightingsAndAssignColors();
-		
-		SemanticHighlightingPresenter.sIsForTest = true;
 		
 		StringBuilder[] testData = TestSourceReader.getContentsForTest(CTestPlugin.getDefault().getBundle(), "ui", getClass(), getName(), 0);
 		
@@ -152,9 +173,8 @@ public class SemanticHighlightingTest extends TestCase {
 		}
 		
 		TestScannerProvider.sIncludeFiles= null;
-		SemanticHighlightingPresenter.sIsForTest = false;
 		
-		restoreAllSemanticHighlightingToDefaults();
+		restorePreferencesToDefaults();
 
 		super.tearDown();
 	}
@@ -190,9 +210,11 @@ public class SemanticHighlightingTest extends TestCase {
 			actual[i] = new ArrayList<String>();
 		}
 		for (Position p : getSemanticHighlightingPositions()) {
-			assertTrue(p instanceof TestHighlightedPosition);
+			assertTrue(p instanceof HighlightedPosition);
+			RGB color = ((HighlightedPosition) p).getHighlighting().getTextAttribute().getForeground().getRGB();
+			assertTrue(fColorToPreferenceKeyMap.containsKey(color));
 			int line = document.getLineOfOffset(p.getOffset());
-			actual[line].add(((TestHighlightedPosition) p).getPreferenceKey());
+			actual[line].add(fColorToPreferenceKeyMap.get(color));
 		}
 		
 		assertEqualMaps(actual, expected);

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/SemanticHighlightingPresenter.java

@@ -239,32 +239,6 @@ public class SemanticHighlightingPresenter implements ITextPresentationListener,
 	/** <code>true</code> iff the current reconcile is canceled. */
 	private boolean fIsCanceled= false;
 
-	/**
-	 * A subclass of HighlightedPosition that records which semantic highlighting
-	 * the position is for. Used for testing.
-	 *
-	 */
-	public static class TestHighlightedPosition extends HighlightedPosition {
-		private String fPreferenceKey;
-		
-		public String getPreferenceKey() {
-			return fPreferenceKey;
-		}
-		
-		public TestHighlightedPosition(int offset, int length, HighlightingStyle highlighting,
-				Object lock, String preferenceKey) {
-			super(offset, length, highlighting, lock);
-			fPreferenceKey = preferenceKey;
-		}
-	}
-
-	/**
-	 * A flag that is set when this class is exercised by a test.
-	 * When the flag is set, the positions added to the document are of type
-	 * TestHighlightedPosition rather than HighlightedPosition.
-	 */
-	public static boolean sIsForTest = false;
-	
 	/**
 	 * Creates and returns a new highlighted position with the given offset, length and highlighting.
 	 * <p>
@@ -278,14 +252,6 @@ public class SemanticHighlightingPresenter implements ITextPresentationListener,
 	 */
 	public HighlightedPosition createHighlightedPosition(int offset, int length, HighlightingStyle style) {
 		// TODO: reuse deleted positions
-		return createHighlightedPosition(offset, length, style, null);
-	}
-	
-	public HighlightedPosition createHighlightedPosition(int offset, int length, HighlightingStyle style,
-			String preferenceKey) {
-		if (sIsForTest) {
-			return new TestHighlightedPosition(offset, length, style, fPositionUpdater, preferenceKey);
-		}
 		return new HighlightedPosition(offset, length, style, fPositionUpdater);
 	}
 	

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/SemanticHighlightingReconciler.java

@@ -184,9 +184,9 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 				SemanticHighlighting semanticHighlighting= fJobSemanticHighlightings[i];
 				if (fJobHighlightings[i].isEnabled() && semanticHighlighting.consumes(fToken)) {
 					if (node instanceof IASTName) {
-						addNameLocation((IASTName) node, i);
+						addNameLocation((IASTName) node, fJobHighlightings[i]);
 					} else {
-						addNodeLocation(node.getFileLocation(), i);
+						addNodeLocation(node.getFileLocation(), fJobHighlightings[i]);
 					}
 					consumed= true;
 					break;
@@ -202,7 +202,7 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 		 * @param name  The name
 		 * @param highlighting The highlighting
 		 */
-		private void addNameLocation(IASTName name, int highlighting) {
+		private void addNameLocation(IASTName name, HighlightingStyle highlightingStyle) {
 			IASTImageLocation imageLocation= name.getImageLocation();
 			if (imageLocation != null) {
 				if (imageLocation.getLocationKind() != IASTImageLocation.MACRO_DEFINITION) {
@@ -211,7 +211,7 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 						int length= imageLocation.getNodeLength();
 						if (offset >= 0 && length > 0) {
 							fMinLocation= offset + length;
-							addPosition(offset, length, highlighting);
+							addPosition(offset, length, highlightingStyle);
 						}
 					}
 				}
@@ -219,7 +219,7 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 				// Fallback in case no image location available.
 				IASTNodeLocation[] nodeLocations= name.getNodeLocations();
 				if (nodeLocations.length == 1 && !(nodeLocations[0] instanceof IASTMacroExpansionLocation)) {
-					addNodeLocation(nodeLocations[0], highlighting);
+					addNodeLocation(nodeLocations[0], highlightingStyle);
 				}
 			}
 		}
@@ -230,7 +230,7 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 		 * @param nodeLocation  The node location
 		 * @param highlighting The highlighting
 		 */
-		private void addNodeLocation(IASTNodeLocation nodeLocation, int highlighting) {
+		private void addNodeLocation(IASTNodeLocation nodeLocation, HighlightingStyle highlightingStyle) {
 			if (nodeLocation == null) {
 				return;
 			}
@@ -239,7 +239,7 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 				int length= nodeLocation.getNodeLength();
 				if (offset > -1 && length > 0) {
 					fMinLocation= offset + length;
-					addPosition(offset, length, highlighting);
+					addPosition(offset, length, highlightingStyle);
 				}
 			}
 		}
@@ -251,15 +251,14 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 		 * @param length The range length
 		 * @param highlighting The highlighting
 		 */
-		private void addPosition(int offset, int length, int highlighting) {
+		private void addPosition(int offset, int length, HighlightingStyle highlightingStyle) {
 			boolean isExisting= false;
 			// TODO: use binary search
-			HighlightingStyle style = fJobHighlightings[highlighting];
 			for (int i= 0, n= fRemovedPositions.size(); i < n; i++) {
 				HighlightedPosition position= fRemovedPositions.get(i);
 				if (position == null)
 					continue;
-				if (position.isEqual(offset, length, style)) {
+				if (position.isEqual(offset, length, highlightingStyle)) {
 					isExisting= true;
 					fRemovedPositions.set(i, null);
 					fNOfRemovedPositions--;
@@ -268,8 +267,7 @@ public class SemanticHighlightingReconciler implements ICReconcilingListener {
 			}
 
 			if (!isExisting) {
-				HighlightedPosition position= fJobPresenter.createHighlightedPosition(offset, length, style,
+				HighlightedPosition position= fJobPresenter.createHighlightedPosition(offset, length, highlightingStyle);
-						fJobSemanticHighlightings[highlighting].getPreferenceKey());
 				fAddedPositions.add(position);
 			}
 		}