chenyuzhi459 commented on pull request #10027: URL: https://github.com/apache/druid/pull/10027#issuecomment-647626491
> @chenyuzhi459 There are more details on the code coverage requirements and how to run the tests locally here - https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md#running-code-coverage-locally > > https://travis-ci.org/github/apache/druid/jobs/699729079 - this job is failing because line 104 in > BackgroundCachePopulator is untested. Maybe you can add a new test class `BackgroundCachePopulatorTest` that adds a test that ensures the onFailure method is called. > > ``` > ------------------------------------------------------------------------------ > org/apache/druid/client/cache/BackgroundCachePopulator.java > ------------------------------------------------------------------------------ > 27 import com.google.common.util.concurrent.ListenableFuture; > 28 import com.google.common.util.concurrent.ListeningExecutorService; > 29 import com.google.common.util.concurrent.MoreExecutors; > 30 F import org.apache.druid.common.guava.GuavaUtils; > 104 F | L GuavaUtils.cancelAll(cacheFutures); > ------------------------------------------------------------------------------ > ``` > > I realize this test coverage was missing before your change, but it would help Druid raise the bar on testing if you are able to add a test for this. > > Hope this helps. Thanks for your tips， it helps me a lot! I have add a unit test for `BackgroundCachePopulator` named `` > @chenyuzhi459 There are more details on the code coverage requirements and how to run the tests locally here - https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md#running-code-coverage-locally > > https://travis-ci.org/github/apache/druid/jobs/699729079 - this job is failing because line 104 in > BackgroundCachePopulator is untested. Maybe you can add a new test class `BackgroundCachePopulatorTest` that adds a test that ensures the onFailure method is called. > > ``` > ------------------------------------------------------------------------------ > org/apache/druid/client/cache/BackgroundCachePopulator.java > ------------------------------------------------------------------------------ > 27 import com.google.common.util.concurrent.ListenableFuture; > 28 import com.google.common.util.concurrent.ListeningExecutorService; > 29 import com.google.common.util.concurrent.MoreExecutors; > 30 F import org.apache.druid.common.guava.GuavaUtils; > 104 F | L GuavaUtils.cancelAll(cacheFutures); > ------------------------------------------------------------------------------ > ``` > > I realize this test coverage was missing before your change, but it would help Druid raise the bar on testing if you are able to add a test for this. > > Hope this helps. Thanks for your tips, it helps me a lot. I have added a unit test named `BackgroundCachePopulatorTest` for `BackgroundCachePopulator` , however, it still fail to pass travis ci test. In https://travis-ci.org/github/apache/druid/jobs/700279328, the line coverage for `BackgroundCachePopulator` is still `0%` ---------------------------------------------------------------- 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: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
