----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9797/#review17557 -----------------------------------------------------------
src/tests/environment.cpp <https://reviews.apache.org/r/9797/#comment37224> Why the TODO instead of fixing it? This is going to break Jenkins if it gets submittted asis? Also, call it CgroupsIsolationModule till we get around to renaming it. src/tests/isolator_tests.cpp <https://reviews.apache.org/r/9797/#comment37225> Reorder these includes. src/tests/isolator_tests.cpp <https://reviews.apache.org/r/9797/#comment37230> why not give it a name? src/tests/isolator_tests.cpp <https://reviews.apache.org/r/9797/#comment37232> How does this file get cleaned up after the test? You should clean this up in TearDown() method of your fixture. src/tests/isolator_tests.cpp <https://reviews.apache.org/r/9797/#comment37233> Could you comment on what these arguments mean? src/tests/isolator_tests.cpp <https://reviews.apache.org/r/9797/#comment37234> I see now why you set the 'id' in utils.hpp. The right way to do this is, do an EXPECT on scheduler's registered call and grab the framework id. src/tests/isolator_tests.cpp <https://reviews.apache.org/r/9797/#comment37237> Umm. This doesn't really test that you are collecting usage from the executor process tree :/ In other words, this test wouldn't fail even if you would've run it with the previous code? src/tests/isolator_tests.cpp <https://reviews.apache.org/r/9797/#comment37238> s/codes/code/ src/tests/utils.hpp <https://reviews.apache.org/r/9797/#comment37228> when a framework doesn't have an id set, the id is given by the master when it first registers. when it fails over, a framework uses this id to re-register with master. tldr; revert this. - Vinod Kone On March 7, 2013, 3:20 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9797/ > ----------------------------------------------------------- > > (Updated March 7, 2013, 3:20 a.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Description > ------- > > Finally some non-external isolation module tests! > > > Diffs > ----- > > src/Makefile.am f7242a7fb55de3d1df4930126345274124331d57 > src/tests/environment.cpp 7c8c2c60ec77c732bf32292088cf5f4cb879ed89 > src/tests/isolator_tests.cpp PRE-CREATION > src/tests/utils.hpp 0d4edad4787f92f86b1486b6b510f46a62d7d541 > > Diff: https://reviews.apache.org/r/9797/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
