Enable failed test logging and fix flaky UT (#910)

* Fix UT and add logging in gradle * Fix UT and add logging in gradle * Simplify test logging config
opendistro-for-elasticsearch · Dec 15, 2020 · 9a2be28 · 9a2be28
1 parent 58ad667
commit 9a2be28
Show file tree

Hide file tree

Showing 8 changed files with 35 additions and 6 deletions.
diff --git a/core/build.gradle b/core/build.gradle
@@ -34,6 +34,7 @@ test {
     useJUnitPlatform()
     testLogging {
         events "passed", "skipped", "failed"
+        exceptionFormat "full"
     }
 }
 

diff --git a/elasticsearch/build.gradle b/elasticsearch/build.gradle
@@ -28,6 +28,7 @@ test {
     useJUnitPlatform()
     testLogging {
         events "passed", "skipped", "failed"
+        exceptionFormat "full"
     }
 }
 

diff --git a/legacy/build.gradle b/legacy/build.gradle
@@ -53,6 +53,11 @@ test {
     // set 'project.projectDir' property to allow unit test classes to access test resources
     // in src/test/resources in current module
     systemProperty('project.root', project.projectDir.absolutePath)
+
+    testLogging {
+        events "passed", "skipped", "failed"
+        exceptionFormat "full"
+    }
 }
 
 dependencies {

diff --git a/ppl/build.gradle b/ppl/build.gradle
@@ -42,6 +42,13 @@ dependencies {
 
 }
 
+test {
+    testLogging {
+        events "passed", "skipped", "failed"
+        exceptionFormat "full"
+    }
+}
+
 jacoco {
     toolVersion = "0.8.5"
 }

diff --git a/protocol/build.gradle b/protocol/build.gradle
@@ -27,6 +27,7 @@ test {
     useJUnitPlatform()
     testLogging {
         events "passed", "skipped", "failed"
+        exceptionFormat "full"
     }
 }
 

diff --git a/sql/build.gradle b/sql/build.gradle
@@ -44,6 +44,7 @@ test {
     useJUnitPlatform()
     testLogging {
         events "passed", "skipped", "failed"
+        exceptionFormat "full"
     }
 }
 

diff --git a/...java/com/amazon/opendistroforelasticsearch/sql/sql/parser/context/QuerySpecification.java b/...java/com/amazon/opendistroforelasticsearch/sql/sql/parser/context/QuerySpecification.java
@@ -40,7 +40,7 @@
 import com.amazon.opendistroforelasticsearch.sql.sql.parser.AstExpressionBuilder;
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -82,7 +82,7 @@ public class QuerySpecification {
    * Aggregate function calls that spreads in SELECT, HAVING clause. Since this is going to be
    * pushed to aggregation operator, de-duplicate is necessary to avoid duplication.
    */
-  private final Set<UnresolvedExpression> aggregators = new HashSet<>();
+  private final Set<UnresolvedExpression> aggregators = new LinkedHashSet<>();
 
   /**
    * Items in GROUP BY clause that may be:

diff --git a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstBuilderTest.java
@@ -197,20 +197,33 @@ public void can_build_where_clause() {
   }
 
   @Test
-  public void can_build_count_star_and_count_literal() {
+  public void can_build_count_literal() {
     assertEquals(
         project(
             agg(
                 relation("test"),
                 ImmutableList.of(
-                    alias("COUNT(*)", aggregate("COUNT", AllFields.of())),
                     alias("COUNT(1)", aggregate("COUNT", intLiteral(1)))),
                 emptyList(),
                 emptyList(),
                 emptyList()),
-            alias("COUNT(*)", aggregate("COUNT", AllFields.of())),
             alias("COUNT(1)", aggregate("COUNT", intLiteral(1)))),
-        buildAST("SELECT COUNT(*), COUNT(1) FROM test"));
+        buildAST("SELECT COUNT(1) FROM test"));
+  }
+
+  @Test
+  public void can_build_count_star() {
+    assertEquals(
+        project(
+            agg(
+                relation("test"),
+                ImmutableList.of(
+                    alias("COUNT(*)", aggregate("COUNT", AllFields.of()))),
+                emptyList(),
+                emptyList(),
+                emptyList()),
+            alias("COUNT(*)", aggregate("COUNT", AllFields.of()))),
+        buildAST("SELECT COUNT(*) FROM test"));
   }
 
   @Test