[GitHub] [druid] ccaominh commented on pull request #10033: Allow append to existing datasources when dynamic partitioning is used

[GitBox <gi...@apache.org>]

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


   > ```
   > 133 F               default boolean sharePartitionSpace(PartialShardSpec partialShardSpec)
   > 134 F               {
   > 135 F | L | B(0/2)    return !partialShardSpec.useNonRootGenerationPartitionSpace();
   > 136                 }
   > ```
   > 
   > The test coverage still complains about this default method, but I believe it's being tested in the new tests I added in [b423e96](https://github.com/apache/druid/commit/b423e964d258d2ba2f9760fa3224026157e98d0d).
   
   I think I know what's going on with the code coverage check:
   
   The coverage check is looking for more unit tests in `core`, since that's where `SharedSpec` lives. `NumberedOverwriteShardSpecTest.testSharePartitionSpace()` is in `core`; however, it is using `NumberedOverwriteShardSpec` and since `OverwriteShardSpec` overrides `sharePartitionSpace()`, the test is not covering `ShardSpec.sharePartitionSpace()`. The other unit tests that were added in https://github.com/apache/druid/commit/b423e964d258d2ba2f9760fa3224026157e98d0d are in `server`, so they're not counted.


----------------------------------------------------------------
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:
users@infra.apache.org



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