neils-dev commented on pull request #2183: URL: https://github.com/apache/ozone/pull/2183#issuecomment-827416873
> I think one point of these unit tests is to test the feature of `BackgroundService.PeriodicTask`, if we create a new PeriodicTask implementation for tests, it seems a little diverged from the goal of these unit tests. > In [HDDS-4231](https://issues.apache.org/jira/browse/HDDS-4231), the waiting for async results is removed, I think we can only focus on solving the intermittent exception here, no need to write the original code for tests. > Besides, the comment of `BackgroundService.PeriodicTask` is not accurate since [HDDS-4231](https://issues.apache.org/jira/browse/HDDS-4231), could you help to update the comment? Thanks for the followup @symious. I understand your comment on extending the `PeriodicTask`, the goal of the unit test in this case is to test deleting blocks - both with the older schema and the block deleting service. The goal of the unit test in these cases I believe is not to exercise the `PeriodicTask` of the `background service`. Further, we are not using the background task in the intended '_periodic_' fashion in these cases as the comment in line 136 implies and how the test operates in a single fashion and _not_ periodic. On the [HDDS-4231] issues, the extended `PeriodicalTaskTestImpl` propagates this LOG.warn as in line 118 so will not change affected callers such as the `TestBlockDeletingService`. -- 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]
