You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Rajat Khandelwal <ra...@gmail.com> on 2015/11/26 11:51:36 UTC
Review Request 40739: LENS-885: Cleanup of Cube test cases
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40739/
-----------------------------------------------------------
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
Re: Review Request 40739: LENS-885: Cleanup of Cube test cases
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40739/#review108297
-----------------------------------------------------------
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java (line 24)
<https://reviews.apache.org/r/40739/#comment167691>
In src/main/, there are some instances where metadata module is importing parse module.
```
lens-cube
org.apache.lens.cube.metadata
CubeColumn.java
import org.apache.lens.cube.parse.TimeRange;
CubeFactTable.java
import org.apache.lens.cube.parse.DateUtil;
ExprColumn.java
import org.apache.lens.cube.parse.HQLParser;
TimePartitionRange.java
import org.apache.lens.cube.parse.DateUtil;
org.apache.lens.cube.metadata.timeline
EndsAndHolesPartitionTimeline.java
import org.apache.lens.cube.parse.TimeRange;
```
Further, `TimeRange` depends on `DateUtil` so if we move `TimeRange`, we'll probably have to partially/completely move `DateUtil` too. `DateUtil` is also used in metadata source.
Bigger challange will be to solve for `ExprColumn` depending on `HQLParser`.
- Rajat Khandelwal
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
>
>
Re: Review Request 40739: LENS-885: Cleanup of Cube test cases
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40739/
-----------------------------------------------------------
(Updated Dec. 11, 2015, 5:01 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 (updated)
-----
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeColumn.java a2a00d2c60077a41eb9576901f1702fd94473cc7
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java d6bfb795373674f77705a2df14a9f24695a76a1a
lens-cube/src/main/java/org/apache/lens/cube/metadata/DateUtil.java PRE-CREATION
lens-cube/src/main/java/org/apache/lens/cube/metadata/TimePartitionRange.java 01069a587c77230bddbae543eba799c2ce495a19
lens-cube/src/main/java/org/apache/lens/cube/metadata/TimeRange.java PRE-CREATION
lens-cube/src/main/java/org/apache/lens/cube/metadata/UpdatePeriod.java 4c76a69d0e2986d450d81b8cbed877995696ba0d
lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/EndsAndHolesPartitionTimeline.java 9d5e264b725ba57ced7618dc2be9de9bcc7983f8
lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java 7f8146158c1e2d883ee9327c4bf847883d0e1004
lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java 9c8b5b9fece146bcf0fa7d5048b93b236cd3d557
lens-cube/src/main/java/org/apache/lens/cube/parse/DateUtil.java cd05c68647e44c1fa96302840330dfd774952e73
lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 200a48cd67e6a5a381ef3723a3adb686d8347efb
lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactHQLContext.java 60b2dde8a7e088d8712879c68cd98193bb72de03
lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java cc8e68c5ad60edeb9638216a6cc9240750e3091e
lens-cube/src/main/java/org/apache/lens/cube/parse/TimeRange.java 7be7ace884bb843604934f650c656e326767b7e3
lens-cube/src/main/java/org/apache/lens/cube/parse/TimerangeResolver.java f772279712c3d1fc5da2325ec2e6a3c1402b550a
lens-cube/src/test/java/org/apache/lens/cube/metadata/CubeFactTableTest.java 2abc6d0b3d9d025543f09ca129bf9d351a188f25
lens-cube/src/test/java/org/apache/lens/cube/metadata/DateFactory.java PRE-CREATION
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e5dbde7af58f74db4ae2a86d9eda602def25d9fe
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestDateUtil.java PRE-CREATION
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/TestAggregateResolver.java 753ca331d28ad594555663d7f03374b75e5760f1
lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java ee84a4cdd5f0a54d90603c1ef96282e8068f4d32
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/TestDateUtil.java ff9a96d2fe4dd8865e1c2a422334ff7f0624420e
lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java d16ea4c56a792f1ac100e398bfdc6c8d1eab4490
lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionContext.java 0d1f9fe878aeec1161329d1ad4e8686fc6d65cd9
lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java 1e21fb0664d7d0153789cd47d14b4989b39bc5bb
lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java ea561b6bd7bba05ebdd33bbe5f9785597459d732
lens-cube/src/test/java/org/apache/lens/cube/parse/TestORTimeRangeWriter.java 4a23818313482d420721d62f908af4cc52f8311d
lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java 255aade5e8b9c4d18ba20e4d263dcce0d497ce76
lens-cube/src/test/java/org/apache/lens/cube/parse/TestRewriterPlan.java 4d3a3dc67ef0f18ef2354ac552fbc4e017022f17
lens-cube/src/test/java/org/apache/lens/cube/parse/TestStorageUtil.java 73c3338f2b44f4d9e878a4de3d424f7184b9f9f0
lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeExtractor.java a4317173f9544ebcab05e290c3129479afd3bd6d
lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java 1fc8bc8ab7f617666b0d105d8c993dd354060c5e
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
lens-server/src/main/java/org/apache/lens/server/query/QueryResultPurger.java 54c657400d11c5eca8ff31ef954edc3c7ee53d1b
Diff: https://reviews.apache.org/r/40739/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 40739: LENS-885: Cleanup of Cube test cases
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40739/
-----------------------------------------------------------
(Updated Dec. 11, 2015, 1:09 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 (updated)
-----
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeColumn.java a2a00d2c60077a41eb9576901f1702fd94473cc7
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java d6bfb795373674f77705a2df14a9f24695a76a1a
lens-cube/src/main/java/org/apache/lens/cube/metadata/DateUtil.java PRE-CREATION
lens-cube/src/main/java/org/apache/lens/cube/metadata/TimePartitionRange.java 01069a587c77230bddbae543eba799c2ce495a19
lens-cube/src/main/java/org/apache/lens/cube/metadata/TimeRange.java PRE-CREATION
lens-cube/src/main/java/org/apache/lens/cube/metadata/UpdatePeriod.java 4c76a69d0e2986d450d81b8cbed877995696ba0d
lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/EndsAndHolesPartitionTimeline.java 9d5e264b725ba57ced7618dc2be9de9bcc7983f8
lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java 7f8146158c1e2d883ee9327c4bf847883d0e1004
lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java 9c8b5b9fece146bcf0fa7d5048b93b236cd3d557
lens-cube/src/main/java/org/apache/lens/cube/parse/DateUtil.java cd05c68647e44c1fa96302840330dfd774952e73
lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 200a48cd67e6a5a381ef3723a3adb686d8347efb
lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactHQLContext.java 60b2dde8a7e088d8712879c68cd98193bb72de03
lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java cc8e68c5ad60edeb9638216a6cc9240750e3091e
lens-cube/src/main/java/org/apache/lens/cube/parse/TimeRange.java 7be7ace884bb843604934f650c656e326767b7e3
lens-cube/src/main/java/org/apache/lens/cube/parse/TimerangeResolver.java f772279712c3d1fc5da2325ec2e6a3c1402b550a
lens-cube/src/test/java/org/apache/lens/cube/metadata/CubeFactTableTest.java 2abc6d0b3d9d025543f09ca129bf9d351a188f25
lens-cube/src/test/java/org/apache/lens/cube/metadata/DateFactory.java PRE-CREATION
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e5dbde7af58f74db4ae2a86d9eda602def25d9fe
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestDateUtil.java PRE-CREATION
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/TestAggregateResolver.java 753ca331d28ad594555663d7f03374b75e5760f1
lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java ee84a4cdd5f0a54d90603c1ef96282e8068f4d32
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/TestDateUtil.java ff9a96d2fe4dd8865e1c2a422334ff7f0624420e
lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java d16ea4c56a792f1ac100e398bfdc6c8d1eab4490
lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionContext.java 0d1f9fe878aeec1161329d1ad4e8686fc6d65cd9
lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java 1e21fb0664d7d0153789cd47d14b4989b39bc5bb
lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java ea561b6bd7bba05ebdd33bbe5f9785597459d732
lens-cube/src/test/java/org/apache/lens/cube/parse/TestORTimeRangeWriter.java 4a23818313482d420721d62f908af4cc52f8311d
lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java 255aade5e8b9c4d18ba20e4d263dcce0d497ce76
lens-cube/src/test/java/org/apache/lens/cube/parse/TestRewriterPlan.java 4d3a3dc67ef0f18ef2354ac552fbc4e017022f17
lens-cube/src/test/java/org/apache/lens/cube/parse/TestStorageUtil.java 73c3338f2b44f4d9e878a4de3d424f7184b9f9f0
lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeExtractor.java a4317173f9544ebcab05e290c3129479afd3bd6d
lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java 1fc8bc8ab7f617666b0d105d8c993dd354060c5e
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
lens-server/src/main/java/org/apache/lens/server/query/QueryResultPurger.java 54c657400d11c5eca8ff31ef954edc3c7ee53d1b
Diff: https://reviews.apache.org/r/40739/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 40739: LENS-885: Cleanup of Cube test cases
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40739/#review109751
-----------------------------------------------------------
Ship it!
Ship It!
- Amareshwari Sriramadasu
On Nov. 30, 2015, 10:05 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40739/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2015, 10:05 a.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/CubeColumn.java a2a00d2c60077a41eb9576901f1702fd94473cc7
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java d6bfb795373674f77705a2df14a9f24695a76a1a
> lens-cube/src/main/java/org/apache/lens/cube/metadata/DateUtil.java PRE-CREATION
> lens-cube/src/main/java/org/apache/lens/cube/metadata/TimePartitionRange.java 01069a587c77230bddbae543eba799c2ce495a19
> lens-cube/src/main/java/org/apache/lens/cube/metadata/TimeRange.java PRE-CREATION
> lens-cube/src/main/java/org/apache/lens/cube/metadata/UpdatePeriod.java 4c76a69d0e2986d450d81b8cbed877995696ba0d
> lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/EndsAndHolesPartitionTimeline.java 9d5e264b725ba57ced7618dc2be9de9bcc7983f8
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java 7f8146158c1e2d883ee9327c4bf847883d0e1004
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java 9c8b5b9fece146bcf0fa7d5048b93b236cd3d557
> lens-cube/src/main/java/org/apache/lens/cube/parse/DateUtil.java 5e17eac62a6b12ef04d1d4e3254efbda54402eb6
> lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 200a48cd67e6a5a381ef3723a3adb686d8347efb
> lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactHQLContext.java 60b2dde8a7e088d8712879c68cd98193bb72de03
> lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java cc8e68c5ad60edeb9638216a6cc9240750e3091e
> lens-cube/src/main/java/org/apache/lens/cube/parse/TimeRange.java 7be7ace884bb843604934f650c656e326767b7e3
> lens-cube/src/main/java/org/apache/lens/cube/parse/TimerangeResolver.java f772279712c3d1fc5da2325ec2e6a3c1402b550a
> lens-cube/src/test/java/org/apache/lens/cube/metadata/CubeFactTableTest.java 2abc6d0b3d9d025543f09ca129bf9d351a188f25
> lens-cube/src/test/java/org/apache/lens/cube/metadata/DateFactory.java PRE-CREATION
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e415a5adeb5a8af16bad56c96b9f9a32cb7afcbb
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestDateUtil.java PRE-CREATION
> 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/TestAggregateResolver.java 753ca331d28ad594555663d7f03374b75e5760f1
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java ee84a4cdd5f0a54d90603c1ef96282e8068f4d32
> 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/TestDateUtil.java ab88fbe067fadc839676f0013dc4885e3fbc5546
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java d16ea4c56a792f1ac100e398bfdc6c8d1eab4490
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionContext.java 0d1f9fe878aeec1161329d1ad4e8686fc6d65cd9
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java 1e21fb0664d7d0153789cd47d14b4989b39bc5bb
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java ea561b6bd7bba05ebdd33bbe5f9785597459d732
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestORTimeRangeWriter.java 4a23818313482d420721d62f908af4cc52f8311d
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java 255aade5e8b9c4d18ba20e4d263dcce0d497ce76
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestRewriterPlan.java 4d3a3dc67ef0f18ef2354ac552fbc4e017022f17
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestStorageUtil.java 73c3338f2b44f4d9e878a4de3d424f7184b9f9f0
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeExtractor.java a4317173f9544ebcab05e290c3129479afd3bd6d
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java 1fc8bc8ab7f617666b0d105d8c993dd354060c5e
> 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
> lens-server/src/main/java/org/apache/lens/server/query/QueryResultPurger.java 54c657400d11c5eca8ff31ef954edc3c7ee53d1b
>
> Diff: https://reviews.apache.org/r/40739/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 40739: LENS-885: Cleanup of Cube test cases
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40739/
-----------------------------------------------------------
(Updated Nov. 30, 2015, 3:35 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 (updated)
-----
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeColumn.java a2a00d2c60077a41eb9576901f1702fd94473cc7
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java d6bfb795373674f77705a2df14a9f24695a76a1a
lens-cube/src/main/java/org/apache/lens/cube/metadata/DateUtil.java PRE-CREATION
lens-cube/src/main/java/org/apache/lens/cube/metadata/TimePartitionRange.java 01069a587c77230bddbae543eba799c2ce495a19
lens-cube/src/main/java/org/apache/lens/cube/metadata/TimeRange.java PRE-CREATION
lens-cube/src/main/java/org/apache/lens/cube/metadata/UpdatePeriod.java 4c76a69d0e2986d450d81b8cbed877995696ba0d
lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/EndsAndHolesPartitionTimeline.java 9d5e264b725ba57ced7618dc2be9de9bcc7983f8
lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java 7f8146158c1e2d883ee9327c4bf847883d0e1004
lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java 9c8b5b9fece146bcf0fa7d5048b93b236cd3d557
lens-cube/src/main/java/org/apache/lens/cube/parse/DateUtil.java 5e17eac62a6b12ef04d1d4e3254efbda54402eb6
lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 200a48cd67e6a5a381ef3723a3adb686d8347efb
lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactHQLContext.java 60b2dde8a7e088d8712879c68cd98193bb72de03
lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java cc8e68c5ad60edeb9638216a6cc9240750e3091e
lens-cube/src/main/java/org/apache/lens/cube/parse/TimeRange.java 7be7ace884bb843604934f650c656e326767b7e3
lens-cube/src/main/java/org/apache/lens/cube/parse/TimerangeResolver.java f772279712c3d1fc5da2325ec2e6a3c1402b550a
lens-cube/src/test/java/org/apache/lens/cube/metadata/CubeFactTableTest.java 2abc6d0b3d9d025543f09ca129bf9d351a188f25
lens-cube/src/test/java/org/apache/lens/cube/metadata/DateFactory.java PRE-CREATION
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e415a5adeb5a8af16bad56c96b9f9a32cb7afcbb
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestDateUtil.java PRE-CREATION
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/TestAggregateResolver.java 753ca331d28ad594555663d7f03374b75e5760f1
lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java ee84a4cdd5f0a54d90603c1ef96282e8068f4d32
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/TestDateUtil.java ab88fbe067fadc839676f0013dc4885e3fbc5546
lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java d16ea4c56a792f1ac100e398bfdc6c8d1eab4490
lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionContext.java 0d1f9fe878aeec1161329d1ad4e8686fc6d65cd9
lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java 1e21fb0664d7d0153789cd47d14b4989b39bc5bb
lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java ea561b6bd7bba05ebdd33bbe5f9785597459d732
lens-cube/src/test/java/org/apache/lens/cube/parse/TestORTimeRangeWriter.java 4a23818313482d420721d62f908af4cc52f8311d
lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java 255aade5e8b9c4d18ba20e4d263dcce0d497ce76
lens-cube/src/test/java/org/apache/lens/cube/parse/TestRewriterPlan.java 4d3a3dc67ef0f18ef2354ac552fbc4e017022f17
lens-cube/src/test/java/org/apache/lens/cube/parse/TestStorageUtil.java 73c3338f2b44f4d9e878a4de3d424f7184b9f9f0
lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeExtractor.java a4317173f9544ebcab05e290c3129479afd3bd6d
lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java 1fc8bc8ab7f617666b0d105d8c993dd354060c5e
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
lens-server/src/main/java/org/apache/lens/server/query/QueryResultPurger.java 54c657400d11c5eca8ff31ef954edc3c7ee53d1b
Diff: https://reviews.apache.org/r/40739/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 40739: LENS-885: Cleanup of Cube test cases
Posted by Rajat Khandelwal <ra...@gmail.com>.
> 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
>
>
Re: Review Request 40739: LENS-885: Cleanup of Cube test cases
Posted by Rajat Khandelwal <ra...@gmail.com>.
> 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.
>
> Rajat Khandelwal wrote:
> 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?
>
> Amareshwari Sriramadasu wrote:
> I see CubeTestSetup is mainly used for cube.parse tests and was lying in cube.parse. package. The main intention i have metadata package should never depend on parse/rewriting package. And i see there is a depedency of test with source class as well. Shall we move Timerange class to metadata, if range is used for timelines aswell? Also, move other required methods from CubeTestSetup into a utility class in cube.metadata package.
>
> Actually, It would be better achived with module separation than through review.
Replied separately.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40739/#review108288
-----------------------------------------------------------
On Nov. 30, 2015, 3:35 p.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40739/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2015, 3:35 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/CubeColumn.java a2a00d2c60077a41eb9576901f1702fd94473cc7
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java d6bfb795373674f77705a2df14a9f24695a76a1a
> lens-cube/src/main/java/org/apache/lens/cube/metadata/DateUtil.java PRE-CREATION
> lens-cube/src/main/java/org/apache/lens/cube/metadata/TimePartitionRange.java 01069a587c77230bddbae543eba799c2ce495a19
> lens-cube/src/main/java/org/apache/lens/cube/metadata/TimeRange.java PRE-CREATION
> lens-cube/src/main/java/org/apache/lens/cube/metadata/UpdatePeriod.java 4c76a69d0e2986d450d81b8cbed877995696ba0d
> lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/EndsAndHolesPartitionTimeline.java 9d5e264b725ba57ced7618dc2be9de9bcc7983f8
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java 7f8146158c1e2d883ee9327c4bf847883d0e1004
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java 9c8b5b9fece146bcf0fa7d5048b93b236cd3d557
> lens-cube/src/main/java/org/apache/lens/cube/parse/DateUtil.java 5e17eac62a6b12ef04d1d4e3254efbda54402eb6
> lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 200a48cd67e6a5a381ef3723a3adb686d8347efb
> lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactHQLContext.java 60b2dde8a7e088d8712879c68cd98193bb72de03
> lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java cc8e68c5ad60edeb9638216a6cc9240750e3091e
> lens-cube/src/main/java/org/apache/lens/cube/parse/TimeRange.java 7be7ace884bb843604934f650c656e326767b7e3
> lens-cube/src/main/java/org/apache/lens/cube/parse/TimerangeResolver.java f772279712c3d1fc5da2325ec2e6a3c1402b550a
> lens-cube/src/test/java/org/apache/lens/cube/metadata/CubeFactTableTest.java 2abc6d0b3d9d025543f09ca129bf9d351a188f25
> lens-cube/src/test/java/org/apache/lens/cube/metadata/DateFactory.java PRE-CREATION
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e415a5adeb5a8af16bad56c96b9f9a32cb7afcbb
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestDateUtil.java PRE-CREATION
> 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/TestAggregateResolver.java 753ca331d28ad594555663d7f03374b75e5760f1
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java ee84a4cdd5f0a54d90603c1ef96282e8068f4d32
> 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/TestDateUtil.java ab88fbe067fadc839676f0013dc4885e3fbc5546
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java d16ea4c56a792f1ac100e398bfdc6c8d1eab4490
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionContext.java 0d1f9fe878aeec1161329d1ad4e8686fc6d65cd9
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java 1e21fb0664d7d0153789cd47d14b4989b39bc5bb
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java ea561b6bd7bba05ebdd33bbe5f9785597459d732
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestORTimeRangeWriter.java 4a23818313482d420721d62f908af4cc52f8311d
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java 255aade5e8b9c4d18ba20e4d263dcce0d497ce76
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestRewriterPlan.java 4d3a3dc67ef0f18ef2354ac552fbc4e017022f17
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestStorageUtil.java 73c3338f2b44f4d9e878a4de3d424f7184b9f9f0
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeExtractor.java a4317173f9544ebcab05e290c3129479afd3bd6d
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java 1fc8bc8ab7f617666b0d105d8c993dd354060c5e
> 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
> lens-server/src/main/java/org/apache/lens/server/query/QueryResultPurger.java 54c657400d11c5eca8ff31ef954edc3c7ee53d1b
>
> Diff: https://reviews.apache.org/r/40739/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 40739: LENS-885: Cleanup of Cube test cases
Posted by Amareshwari Sriramadasu <am...@apache.org>.
> On Nov. 30, 2015, 5:54 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.
>
> Rajat Khandelwal wrote:
> 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?
I see CubeTestSetup is mainly used for cube.parse tests and was lying in cube.parse. package. The main intention i have metadata package should never depend on parse/rewriting package. And i see there is a depedency of test with source class as well. Shall we move Timerange class to metadata, if range is used for timelines aswell? Also, move other required methods from CubeTestSetup into a utility class in cube.metadata package.
Actually, It would be better achived with module separation than through review.
- Amareshwari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40739/#review108288
-----------------------------------------------------------
On Nov. 26, 2015, 10:51 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40739/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2015, 10:51 a.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
>
>
Re: Review Request 40739: LENS-885: Cleanup of Cube test cases
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40739/#review108288
-----------------------------------------------------------
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java (line 24)
<https://reviews.apache.org/r/40739/#comment167664>
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.
- Amareshwari Sriramadasu
On Nov. 26, 2015, 10:51 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40739/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2015, 10:51 a.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
>
>