----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38521/#review99944 -----------------------------------------------------------
lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java (line 61) <https://reviews.apache.org/r/38521/#comment156971> Are we missing out on some scenarios by removing one storage? lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java (line 456) <https://reviews.apache.org/r/38521/#comment156972> Same thing here, do we need to remove storages? lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java (lines 306 - 308) <https://reviews.apache.org/r/38521/#comment156973> can use `expectedJoinList.containsAll(actualQueryParts)` instead of the loop. And the check should also be done in the reverse order. lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java (line 311) <https://reviews.apache.org/r/38521/#comment156977> Instead of making a new method, I'd say make a class `Query`. Then in the old compare method, take two strings and make to objects. Inside the constructor, the clauses can be extracted. Then `equals` can be defined on that class. that combined with `toString` will give nice output when we directly use `Assert.assertEquals(query1, query2)` instead of doing all string processing outside. lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java (line 316) <https://reviews.apache.org/r/38521/#comment156974> If one query has inner join and the other doesn't, then also our asserts will pass. lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java (line 86) <https://reviews.apache.org/r/38521/#comment156978> is it needed? lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java (line 161) <https://reviews.apache.org/r/38521/#comment156984> For existing test cases doing this might seem fine, but when someone is writing a new test case, we can't expect them to 1. either write test cases like this. (split and set check for each new test case) 2. or run with both java 7 and java 8 just to figure out there'll be a difference and needs special handling It would be better if the underlying structure supports comparison so that the comparison logic is not: 1. outside of the structure and 2. repeated several times lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java (line 44) <https://reviews.apache.org/r/38521/#comment156985> Removing c2? lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java (line 292) <https://reviews.apache.org/r/38521/#comment156986> only c2? what was the default? lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java (line 321) <https://reviews.apache.org/r/38521/#comment156987> same here. lens-cube/src/test/java/org/apache/lens/cube/parse/TestStorageUtil.java (line 202) <https://reviews.apache.org/r/38521/#comment156988> Is some behaviour different in java 8 than java 7? Or is there some other code change that requires this portion to be commented? lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/ColumnarSQLRewriter.java (lines 116 - 129) <https://reviews.apache.org/r/38521/#comment156989> +1 lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestColumnarSQLRewriter.java (line 88) <https://reviews.apache.org/r/38521/#comment156990> This `compareJoinQueries` is different than the other `compareJoinQueries`? lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestColumnarSQLRewriter.java (line 446) <https://reviews.apache.org/r/38521/#comment156991> +1 for changing underlying structures so that both java7 and java8 behave in similar manner. - Rajat Khandelwal On Sept. 22, 2015, 6:49 a.m., Raju Bairishetti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38521/ > ----------------------------------------------------------- > > (Updated Sept. 22, 2015, 6:49 a.m.) > > > Review request for lens, Amareshwari Sriramadasu and Rajat Khandelwal. > > > Bugs: LENS-607 > https://issues.apache.org/jira/browse/LENS-607 > > > Repository: lens > > > Description > ------- > > Iterators in java7 and java8 are returning elements in different order. All > most all failures are because of this case. > > Changes: > TestBaseCubeQueries: Changed driver supported storages to single storage > instead of multiple > TestCubeRewriter: Returning diffrent orders in the joins. Separated join part > from the actaul query part and verified join part and remaining query part > separately. > TestStorageUtil: Multiple sets can answer the given partitions. Changed > asserts accordingly. > TestTimeRangeResolver: Changed asserts to cover all the errors. > > *ColumnSQLRewriter* : Changed all data structures to *Linked* datastructures > (Arraylist --> LinkedList, HashSet to LinkedHashSet, HashMap to > LinkedHashMap) to return the elements in insertion order. > > TestColumnSQLRewriter: Changed the order of columns in queries. > > > Diffs > ----- > > lens-cli/src/test/java/org/apache/lens/cli/TestLensCubeCommands.java > 39441c9 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java > 0f76c76 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java > a58f5fe > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java > bde4edd > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java > 493b8d6 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestStorageUtil.java > 81f515b > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java > cb27d50 > > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/ColumnarSQLRewriter.java > 295b476 > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestColumnarSQLRewriter.java > db09a4b > > Diff: https://reviews.apache.org/r/38521/diff/ > > > Testing > ------- > > Tested on both java7 and java8. Build is successful. Will post the build > artifact summary soon. > > c1mng0pxdty3:lens raju.bairishetti$ java -version > **java version "1.8.0_40"** > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules .............................. SUCCESS [ 4.849 > s] > [INFO] Lens ............................................... SUCCESS [ 5.806 > s] > [INFO] Lens API ........................................... SUCCESS [ 31.562 > s] > [INFO] Lens API for server and extensions ................. SUCCESS [ 23.200 > s] > [INFO] Lens Cube .......................................... SUCCESS [06:19 > min] > [INFO] Lens DB storage .................................... SUCCESS [ 23.015 > s] > [INFO] Lens Query Library ................................. SUCCESS [ 18.249 > s] > [INFO] Lens Hive Driver ................................... SUCCESS [03:11 > min] > [INFO] Lens Driver for JDBC ............................... SUCCESS [ 41.094 > s] > [INFO] Lens Elastic Search Driver ......................... SUCCESS [ 26.188 > s] > [INFO] Lens Server ........................................ SUCCESS [12:38 > min] > [INFO] Lens client ........................................ SUCCESS [ 44.951 > s] > [INFO] Lens CLI ........................................... SUCCESS [04:57 > min] > [INFO] Lens Examples ...................................... SUCCESS [ 11.491 > s] > [INFO] Lens Distribution .................................. SUCCESS [ 19.483 > s] > [INFO] Lens ML Lib ........................................ SUCCESS [02:35 > min] > [INFO] Lens ML Ext Distribution ........................... SUCCESS [ 7.919 > s] > [INFO] Lens Regression .................................... SUCCESS [ 11.523 > s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 34:13 min > [INFO] Finished at: 2015-09-19T12:13:27+05:30 > [INFO] Final Memory: 158M/1483M > [INFO] > ------------------------------------------------------------------------ > > > JDK-7: **sun-jdk-1.7.0_55** > > regression-2.4.0-beta-SNAPSHOT-tests.jar > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules ............................. SUCCESS [1.999s] > [INFO] Lens .............................................. SUCCESS [2.816s] > [INFO] Lens API .......................................... SUCCESS [23.685s] > [INFO] Lens API for server and extensions ................ SUCCESS [20.093s] > [INFO] Lens Cube ......................................... SUCCESS [5:02.073s] > [INFO] Lens DB storage ................................... SUCCESS [19.937s] > [INFO] Lens Query Library ................................ SUCCESS [15.858s] > [INFO] Lens Hive Driver .................................. SUCCESS [2:50.353s] > [INFO] Lens Driver for JDBC .............................. SUCCESS [37.076s] > [INFO] Lens Elastic Search Driver ........................ SUCCESS [15.348s] > [INFO] Lens Server ....................................... SUCCESS [5:42.566s] > [INFO] Lens client ....................................... SUCCESS [34.983s] > [INFO] Lens CLI .......................................... SUCCESS [2:34.585s] > [INFO] Lens Examples ..................................... SUCCESS [8.688s] > [INFO] Lens Distribution ................................. SUCCESS [8.809s] > [INFO] Lens ML Lib ....................................... SUCCESS [1:28.010s] > [INFO] Lens ML Ext Distribution .......................... SUCCESS [2.393s] > [INFO] Lens Regression ................................... SUCCESS [14.258s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 21:04.437s > [INFO] Finished at: Sat Sep 19 06:48:15 UTC 2015 > [INFO] Final Memory: 178M/1113M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Raju Bairishetti > >
