ccaominh commented on pull request #10033:
URL: https://github.com/apache/druid/pull/10033#issuecomment-649750336


   Looking at the code coverage issue a bit more:
   
   All the ShardSpecs live in `core`:
   ```
   find . -name "*ShardSpec.java"
   
   
./core/src/main/java/org/apache/druid/timeline/partition/NumberedOverwriteShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/OverwriteShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/RangeBucketShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/NumberedPartialShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/HashBucketShardSpec.java
   ./core/src/main/java/org/apache/druid/timeline/partition/NoneShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/SingleDimensionShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/NumberedShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/HashBasedNumberedPartialShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/HashBasedNumberedShardSpec.java
   ./core/src/main/java/org/apache/druid/timeline/partition/LinearShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/SingleDimensionPartialShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/NumberedOverwritePartialShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/BucketNumberedShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/BuildingHashBasedNumberedShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/BuildingNumberedShardSpec.java
   ./core/src/main/java/org/apache/druid/timeline/partition/ShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/PartialShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/LinearPartialShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/BuildingSingleDimensionShardSpec.java
   
./core/src/main/java/org/apache/druid/timeline/partition/BuildingShardSpec.java
   
./server/src/main/java/org/apache/druid/segment/realtime/appenderator/SegmentIdWithShardSpec.java
   
./indexing-hadoop/src/main/java/org/apache/druid/indexer/HadoopyShardSpec.java
   ```
   
   But some of their unit tests live in `server` (e.g., 
`SingleDimensionShardSpecTest`):
   ```
   find . -name "*ShardSpecTest.java"
   
   
./core/src/test/java/org/apache/druid/timeline/partition/NumberedOverwritePartialShardSpecTest.java
   
./core/src/test/java/org/apache/druid/timeline/partition/NoneShardSpecTest.java
   
./core/src/test/java/org/apache/druid/timeline/partition/BuildingSingleDimensionShardSpecTest.java
   
./core/src/test/java/org/apache/druid/timeline/partition/BuildingHashBasedNumberedShardSpecTest.java
   
./core/src/test/java/org/apache/druid/timeline/partition/HashBucketShardSpecTest.java
   
./core/src/test/java/org/apache/druid/timeline/partition/SingleDimensionPartialShardSpecTest.java
   
./core/src/test/java/org/apache/druid/timeline/partition/NumberedOverwriteShardSpecTest.java
   
./core/src/test/java/org/apache/druid/timeline/partition/NumberedPartialShardSpecTest.java
   
./core/src/test/java/org/apache/druid/timeline/partition/HashBasedNumberedPartialShardSpecTest.java
   
./core/src/test/java/org/apache/druid/timeline/partition/BuildingNumberedShardSpecTest.java
   
./core/src/test/java/org/apache/druid/timeline/partition/RangeBucketShardSpecTest.java
   
./server/src/test/java/org/apache/druid/segment/realtime/appenderator/SegmentIdWithShardSpecTest.java
   
./server/src/test/java/org/apache/druid/server/shard/NumberedShardSpecTest.java
   
./server/src/test/java/org/apache/druid/server/shard/SingleDimensionShardSpecTest.java
   
./server/src/test/java/org/apache/druid/timeline/partition/HashBasedNumberedShardSpecTest.java
   ```
   
   If those unit tests had been in `core` instead of `server` then the coverage 
check for this PR would have passed since the relevant unit tests were added to 
`SingleDimensionShardSpecTest`, for example.
   
   I suggest we do a followup PR to move the `ShardSpec` tests from `server` to 
`core` and proceed with merging this PR, since the coverage check failure is a 
result of the prior misplacement of test classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org

Reply via email to