Github user kirklund commented on the issue:

    https://github.com/apache/incubator-geode/pull/296
  
    We've been requiring unit tests or integration tests for all bug fixes. 
    
    At a minimum, I would recommend adding a new test that fails due to at 
least of the conditions reported by GEODE-2109 and then passes with your 
changes. Example:
    `
    import static org.assertj.core.api.Assertions.assertThat; 
    
    import org.apache.geode.test.junit.categories.UnitTest;
    import org.junit.Rule;
    import org.junit.Test;
    import org.junit.contrib.java.lang.system.SystemErrRule;
    import org.junit.experimental.categories.Category;
    
    @Category(UnitTest.class)
    public class FederatingManagerSubmitTaskWithFailureTest {
    
      private FederatingManager manager;
    
      @Rule
      public SystemErrRule systemErrRule = new SystemErrRule().enableLog();
    
      @Test
      public void submittedTaskShouldLogFailure() throws Exception {
        manager.submitTask(() -> {throw new Exception("error message");});
        
assertThat(systemErrRule.getLog()).contains(Exception.class.getName()).contains("error
 message");
      }
    }
    `
    The above test would require a couple things:
    1) some sort of @Before method which instantiates manager
    2) change FederatingManager.submitTask(...) to package scope so the unit 
test can call it (right now FederatingManager is not a test-friendly class)



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to