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