----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40369/#review106920 -----------------------------------------------------------
gemfire-core/src/test/java/com/gemstone/gemfire/management/DistributedSystemDUnitTest.java (line 173) <https://reviews.apache.org/r/40369/#comment165726> To test your fix don't you need to set the alert level to warning and then immediately to severe and then check and make sure everyone has it set to severe and not warning? Since the setAlertLevel message sleeps for 10 seconds and I you need to refactor this test a bit to have this every happen. I could be wrong but it doesn't seem like this test would have failed before you fixed the message to be serial. Also it doesn't see like you want to call warnLevelAlert and severeLevelAlert between the calls of setAlertLevel since these also take some time (they generate log messages). gemfire-core/src/test/java/com/gemstone/gemfire/management/DistributedSystemDUnitTest.java (line 184) <https://reviews.apache.org/r/40369/#comment165722> Something I don't see in this diff is when this line gets called: setAlertLevel(managingNode, AlertDetails.getAlertLevelAsString(Alert.OFF)); It seems like this test should have this in a finally block to make sure we attempt to turn alerting back off. - Darrel Schneider On Nov. 16, 2015, 1:48 p.m., Jens Deppe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40369/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2015, 1:48 p.m.) > > > Review request for geode and Dan Smith. > > > Repository: geode > > > Description > ------- > > - Updating the alert level is changed from a pooled message to a serial > message. This ensures that consecutive alert level changes are > processed in the correct order. > > Also reworked the test so that we're not trying to set the same alert level > twice in a row otherwise it becomes difficult to know where the messages have > been received. > > > Diffs > ----- > > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/admin/remote/AlertLevelChangeMessage.java > 7e6c0a4ef08cb53f16d7b024778625fbeb976cca > > gemfire-core/src/test/java/com/gemstone/gemfire/management/DistributedSystemDUnitTest.java > 193dd1222e235311413ccbb89a00ecf43e657de1 > > Diff: https://reviews.apache.org/r/40369/diff/ > > > Testing > ------- > > Ran DistributedSystemDUnitTest > > > Thanks, > > Jens Deppe > >
