[GitHub] [accumulo] wx930910 opened a new issue #2253: Refactor MockRateLimiter to improve test design

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

wx930910 opened a new issue #2253:
URL: https://github.com/apache/accumulo/issues/2253


   ### Description
   
   I noticed that there is a test class [MockRateLimiter](https://github.com/apache/accumulo/blob/1dc72fce2c781dee597c8c11876a3bc6c321c199/core/src/test/java/org/apache/accumulo/core/file/streams/MockRateLimiter.java#L25) implements a production interface [RateLimiter](https://github.com/apache/accumulo/blob/1dc72fce2c781dee597c8c11876a3bc6c321c199/core/src/main/java/org/apache/accumulo/core/util/ratelimit/RateLimiter.java#L21) to assist testing production function [RateLimitedOutputStream.write(byte[], int, int)](https://github.com/apache/accumulo/blob/1dc72fce2c781dee597c8c11876a3bc6c321c199/core/src/main/java/org/apache/accumulo/core/file/streams/RateLimitedOutputStream.java#L47) and [RateLimitedInputStream.read(byte[], int, int)](https://github.com/apache/accumulo/blob/1dc72fce2c781dee597c8c11876a3bc6c321c199/core/src/main/java/org/apache/accumulo/core/file/streams/RateLimitedInputStream.java#L51). This might not be the best priactice in unit testing and can be improved by levera
 ging mocking frameworks. 
   
   ### Current Implementation
   - `MockRateLimiter` use AtomicLong variable to keep tracking excution times of [RateLimiter:acquire(long)](https://github.com/apache/accumulo/blob/1dc72fce2c781dee597c8c11876a3bc6c321c199/core/src/main/java/org/apache/accumulo/core/util/ratelimit/RateLimiter.java#L28).
   - Use assertion statements in unit tests to verify the behavior of the `MockRateLimiter` to make sure `read` and `write` function excuted as desired.
   
   ### Proposed Implementation
   - Replace `MockRateLimiter` with a mocking object created by Mockito.
   - Use method stub to control the behavior of the mocking object.
   - Extrac variables in test case to make test condition more explict.
   
   ### Motivation
   
   - Decoupling test class `MockRateLimiter` from production class `RateLimiter`.
   - Making test condition more clear by verifying mocking obejct's behavior with a local variable.
   - Making test logic more clear by using method stub instead of method overriding.
   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

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

[GitHub] [accumulo] milleruntime closed issue #2253: Refactor MockRateLimiter to improve test design

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

milleruntime closed issue #2253:
URL: https://github.com/apache/accumulo/issues/2253


   


-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

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