Merge remote-tracking branch 'origin/release' into integration/master

zanata-war/src/main/java/org/zanata/webtrans/shared/validation/action/JavaVariablesValidation.java

@@ -21,6 +21,7 @@
 package org.zanata.webtrans.shared.validation.action;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map.Entry;
@@ -37,8 +38,7 @@
  * each argument index is used with the same FormatType.
  *
  * @author David Mason, damason@redhat.com
- * @see http
- *      ://docs.oracle.com/javase/1.4.2/docs/api/java/text/MessageFormat.html
+ * @see http://docs.oracle.com/javase/7/docs/api/java/text/MessageFormat.html
  **/
 public class JavaVariablesValidation extends AbstractValidationAction {
     public JavaVariablesValidation(ValidationId id, ValidationMessages messages) {
@@ -89,6 +89,14 @@ public List<String> doValidate(String source, String target) {
             }
         }
 
+        // Sort variable lists to ensure consistent ordering of variables
+        // in error messages:
+        Collections.sort(missing);
+        Collections.sort(missingQuoted);
+        Collections.sort(added);
+        Collections.sort(addedQuoted);
+        Collections.sort(different);
+
         boolean looksLikeMessageFormatString = !sourceInfo.varCounts.isEmpty();
 
         if (!missing.isEmpty()) {

zanata-war/src/test/java/org/zanata/service/impl/TranslationFinderTest.java

@@ -5,6 +5,9 @@
 import static org.junit.Assert.*;
 import static org.zanata.service.impl.ExecutionHelper.cartesianProduct;
 
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -73,6 +76,7 @@ public void before() throws Exception {
                 .use("session", new FullTextSessionImpl(getSession()))
                 .useImpl(IndexingServiceImpl.class)
                 .ignoreNonResolvable();
+        // TODO this is too expensive to do 64 times!
         seam.autowire(SearchIndexManager.class).reindex(true, true, false);
         LocaleDAO localeDAO = seam.autowire(LocaleDAO.class);
 
@@ -143,13 +147,17 @@ public void individualTest() {
     }
 
     @Test
+    // currently 64 combinations
+    // TODO reduce the combinations, or reduce the cost per execution
     @UseDataProvider("copyTransCombinations")
     public void testTextFlowTargetDAO(Execution execution) {
         testExecution(TextFlowTargetDAO.class, execution);
     }
 
 
     @Test
+    // currently 64 combinations
+    // TODO reduce the combinations, or reduce the cost per execution
     @UseDataProvider("copyTransCombinations")
     public void testTranslationMemoryServiceImpl(Execution execution) {
         testExecution(TranslationMemoryServiceImpl.class, execution);
@@ -224,6 +232,7 @@ private void testExecution(Class<? extends TranslationFinder> impl, Execution ex
     }
 
     @DataProvider
+    // currently 64 combinations
     public static Object[][] copyTransCombinations() {
         Set<Execution> expandedExecutions = generateAllExecutions();
 
@@ -240,17 +249,25 @@ private static Set<Execution> generateAllExecutions() {
         Set<Execution> allExecutions =
                 new HashSet<Execution>();
         List<Boolean> booleans = asList(true, false);
-        Set<Object[]> paramsSet =
-                cartesianProduct(booleans, booleans, booleans, booleans,
-                        booleans, booleans, booleans);
-
-        for (Object[] params : paramsSet) {
-            Execution exec =
-                    new Execution(
-                            (Boolean) params[0], (Boolean) params[1],
-                            (Boolean) params[2], (Boolean) params[3],
-                            (Boolean) params[4], (Boolean) params[5]);
-            allExecutions.add(exec);
+        // NB 2 ^ 6 = 64 combinations
+        Iterable[] colls = { booleans, booleans, booleans, booleans,
+                booleans, booleans };
+        try {
+            // The use of reflection is a little clumsy, but it helps
+            // to ensure that we get the number of constructor arguments
+            // correct for the cartesian product generator.
+            Class[] paramTypes = new Class[colls.length];
+            Arrays.fill(paramTypes, Boolean.TYPE);
+            Constructor ctor =
+                    Execution.class.getConstructor(paramTypes);
+            Set<Object[]> paramsSet = cartesianProduct(colls);
+            for (Object[] params : paramsSet) {
+                Execution exec =
+                        (Execution) ctor.newInstance(params);
+                allExecutions.add(exec);
+            }
+        } catch (NoSuchMethodException | InstantiationException | InvocationTargetException | IllegalAccessException e) {
+            throw new RuntimeException(e);
         }
         return allExecutions;
     }

zanata-war/src/test/java/org/zanata/webtrans/shared/validation/JavaVariablesValidationTest.java

@@ -93,8 +93,8 @@ public void missingVarsThroughoutTarget() {
         List<String> errorList =
                 javaVariablesValidation.validate(source, target);
 
-        assertThat(errorList, hasItem(messages.varsMissing(Arrays.asList("{2}",
-                "{1}", "{0}"))));
+        assertThat(errorList, hasItem(messages.varsMissing(Arrays.asList("{0}",
+                "{1}", "{2}"))));
         assertThat(errorList.size(), is(1));
     }
 
@@ -117,7 +117,7 @@ public void addedVarsThroughoutTarget() {
                 javaVariablesValidation.validate(source, target);
 
         assertThat(errorList,
-                hasItem(messages.varsAdded(Arrays.asList("{2}", "{1}", "{0}"))));
+                hasItem(messages.varsAdded(Arrays.asList("{0}", "{1}", "{2}"))));
         assertThat(errorList.size(), is(1));
     }
 
@@ -144,8 +144,8 @@ public void disturbanceInTheForce() {
         List<String> errorList =
                 javaVariablesValidation.validate(source, target);
 
-        assertThat(errorList, hasItem(messages.varsMissing(Arrays.asList("{2}",
-                "{1}", "{0}"))));
+        assertThat(errorList, hasItem(messages.varsMissing(Arrays.asList("{0}",
+                "{1}", "{2}"))));
         assertThat(errorList.size(), is(1));
     }
 
@@ -157,7 +157,7 @@ public void diskContainsFiles() {
                 javaVariablesValidation.validate(source, target);
 
         assertThat(errorList,
-                hasItem(messages.varsMissing(Arrays.asList("{1}", "{0}"))));
+                hasItem(messages.varsMissing(Arrays.asList("{0}", "{1}"))));
         assertThat(errorList.size(), is(1));
     }