> On Nov. 25, 2013, 7:07 p.m., Ben Mahler wrote: > > src/tests/fault_tolerance_tests.cpp, line 1384 > > <https://reviews.apache.org/r/15773/diff/1/?file=389617#file389617line1384> > > > > Ok, I like that this summary mentions what it is testing, but it does > > not mention how it is testing it, so I must read through the test and > > observe that you're starting a master, slave, framework, and then a second > > framework, dropping a message, using a kill task and a finished status to > > ensure the kill task is ignored. > > > > I think this is difficult to extract from reading the tests for those > > who are less well versed in our integration testing abstractions, so the > > numbered steps are nice for them :) > > > > I tried to follow this format here: > > https://reviews.apache.org/r/15778/diff/#1.2 > > Vinod Kone wrote: > I'm not convinced about repeating the specific details of "how" in the > comment when it is already in the comments. I think our abstracts are pretty > easy to understand what's happening (StartMaster(), StartSlave(), > driver.start() etc). > > That said I added couple more comments in the test to make you happy :)
Appreciate it! Although I still don't see a summary about what this test is doing ;) Think of this as a summary rather than repetition, sometimes we have to think pretty hard about how to test things that it's good to summarize this for others trying to understand the test. In complicated tests, having both what is being tested, along with how it's being tested at the top of the test seems useful, no? - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15773/#review29380 ----------------------------------------------------------- On Nov. 25, 2013, 7:46 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15773/ > ----------------------------------------------------------- > > (Updated Nov. 25, 2013, 7:46 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Bugs: MESOS-834 > https://issues.apache.org/jira/browse/MESOS-834 > > > Repository: mesos-git > > > Description > ------- > > See bug for details. > > > Diffs > ----- > > src/master/master.hpp c86c1f1e7eb23c168bcd2e8ab0a0701c421fc59d > src/master/master.cpp a08d01208ff7bbb878b2d50d8406efee4de86171 > src/tests/fault_tolerance_tests.cpp > 6cb5829119ed3e855740fb9331e4fb19c16d3629 > > Diff: https://reviews.apache.org/r/15773/diff/ > > > Testing > ------- > > make check (Linux and OSX) > > sudo ./bin/mesos-tests.sh > --gtest_filter="*IgnoreKillTaskFromUnregisteredFramework*" > --gtest_repeat=1000 --gtest_break_on_failure > > > Thanks, > > Vinod Kone > >
