> On Nov. 30, 2015, 11:24 a.m., Amareshwari Sriramadasu wrote: > > lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java, > > line 24 > > <https://reviews.apache.org/r/40739/diff/1/?file=1147414#file1147414line24> > > > > Having cube.parse dependency from cube.metadata does not seem correct. > > > > If the required methods can be moved to a utility class in > > cube.metadata and used by both would be good.
It's only test code. Besides, we already have such a dependency. https://github.com/apache/lens/blob/master/lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java#L24, https://github.com/apache/lens/blob/master/lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java#L38 Thirdly, CubeTestSetup is the origin of most cube test cases. More than a few classes will depend on it to provide constants, static functions etc. Are you saying the original layout needs to be changed? If yes, can you elaborate further? - Rajat ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40739/#review108288 ----------------------------------------------------------- On Nov. 26, 2015, 4:21 p.m., Rajat Khandelwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40739/ > ----------------------------------------------------------- > > (Updated Nov. 26, 2015, 4:21 p.m.) > > > Review request for lens. > > > Bugs: LENS-885 > https://issues.apache.org/jira/browse/LENS-885 > > > Repository: lens > > > Description > ------- > > * Some functions are wrongly placed. e.g. DateUtil.getCeilDate(date, > updatePeriod) can be better expressed as updatePeriod.getCeilDate(date). > * Date parsing from query can be cached > * There is a bloat of Date variables in CubeTestSetup, organize them and only > keep named variables for the most used ones > * Unify time range clause creation across test cases. Right now, tests create > time ranges as and when needed. Remove repetition and redundancies. > * Correct names to variables > > > Diffs > ----- > > lens-cube/src/main/java/org/apache/lens/cube/metadata/UpdatePeriod.java > 4c76a69d0e2986d450d81b8cbed877995696ba0d > lens-cube/src/main/java/org/apache/lens/cube/parse/DateUtil.java > 5e17eac62a6b12ef04d1d4e3254efbda54402eb6 > > lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java > e415a5adeb5a8af16bad56c96b9f9a32cb7afcbb > lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java > 1357035c8dfc6a5115c02d8fdc7cd8bb341eb6dd > > lens-cube/src/test/java/org/apache/lens/cube/parse/FieldsCannotBeQueriedTogetherTest.java > 0fea9f1bb4a266b680a252786ebd9e931d1b0c09 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestBetweenTimeRangeWriter.java > 9a2493c306938a0823d4b7d2bd264f915143ea64 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java > fea70b72cbde190d69d46f68d163fd9e541c53f8 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java > d16ea4c56a792f1ac100e398bfdc6c8d1eab4490 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestORTimeRangeWriter.java > 4a23818313482d420721d62f908af4cc52f8311d > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeExtractor.java > a4317173f9544ebcab05e290c3129479afd3bd6d > lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeWriter.java > 0248409123f0588cbd5b45d0894bc9bf3503c888 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeWriterWithQuery.java > 7bd7b6b962fd9be83309ea3fc870dee9aefc2895 > > Diff: https://reviews.apache.org/r/40739/diff/ > > > Testing > ------- > > > Thanks, > > Rajat Khandelwal > >
