> On Feb. 23, 2017, 10:18 p.m., Kirk Lund wrote:
> > +1 I know this change is good and would be safe commit it as is, however we 
> > have been trying to create UnitTests for any class that we touch that 
> > doesn't already have a UnitTest. As a whole, the project relies too much on 
> > end-to-end tests which take forever to run and tend to be more flaky.
> > 
> > So I still recommend trying to write a UnitTest that at a minimum covers 
> > the method you've modified. My definition of UnitTest here is a very fast 
> > test that uses mocking (Mockito) to mock everything this class method 
> > interacts with and the test does not interact with file system or network. 
> > If this method interacts with file system, then there are generally two 
> > choices: test it in an IntegrationTest or refactor the method to allow you 
> > to UnitTest it in isolation from file system. The test class itself has a 
> > JUnit @Category annotation at the top and  the category is either 
> > UnitTest.class or IntegrationTest.class in this case. 
> > 
> > For testing an exception, I recommend using AssertJ:
> > 
> > assertThatThrownBy(() -> 
> > target.callMethodThatThrows()).isInstanceOf(IllegalStateException.class).hasMessageContaining("resourceTypeId").hasMessageContaining("resourceName");
> > 
> > You could instead use .hasMessage("ResourceType is missing for 
> > resourceTypeId xxx, resourceName yyy") but that does tend to make for a 
> > more brittle test.

In this case, you could probably just modify 
StatArchiveWithMissingResourceTypeRegressionTest


- Kirk


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56964/#review166590
-----------------------------------------------------------


On Feb. 23, 2017, 3:03 a.m., Srikanth Manvi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56964/
> -----------------------------------------------------------
> 
> (Updated Feb. 23, 2017, 3:03 a.m.)
> 
> 
> Review request for geode and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Enhanced the log statement to include the ResourceType Name.
> 
> 
> Diffs
> -----
> 
>   
> geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveReader.java
>  9fba511 
> 
> Diff: https://reviews.apache.org/r/56964/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Srikanth Manvi
> 
>

Reply via email to