> On March 25, 2015, 9:24 p.m., Jie Yu wrote: > > src/tests/memory_test_helper.cpp, lines 214-215 > > <https://reviews.apache.org/r/31276/diff/6/?file=904736#file904736line214> > > > > YOu are not checking the result of getline. This is not a good > > programming practice. Any reason not using: > > ``` > > while (getline(cin, line) >= 0) { > > .... > > } > > > > if (!cin.eof()) { > > cerr << "" << endl; > > return 1; > > } > > > > return 0; > > ```
dropped as discussed. > On March 25, 2015, 9:24 p.m., Jie Yu wrote: > > src/tests/memory_test_helper.cpp, line 160 > > <https://reviews.apache.org/r/31276/diff/6/?file=904736#file904736line160> > > > > Move this down after validation is done. > > > > Also, s/memory/buffer/ that won't change the function frame size though? Is it the size you concern, or the style? If it's the latter, I'd move both lines down after validation? > On March 25, 2015, 9:24 p.m., Jie Yu wrote: > > src/tests/memory_test_helper.cpp, line 148 > > <https://reviews.apache.org/r/31276/diff/6/?file=904736#file904736line148> > > > > Since allocateRSS is only used here, please kill the helper funciton > > and move the logic into increaseRSS This is very heavily used in the following patches for stat tests, RSS, pagecache, mmap, activeanon and writeback. Let's hold on to it?? The end result is I'd need it split out. > On March 25, 2015, 9:24 p.m., Jie Yu wrote: > > src/tests/cgroups_tests.cpp, line 1086 > > <https://reviews.apache.org/r/31276/diff/6/?file=904734#file904734line1086> > > > > Can you set the limit in the test? do you mean move this to be a test case field instead of a fixture field? - Chi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31276/#review77779 ----------------------------------------------------------- On March 24, 2015, 10:15 p.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31276/ > ----------------------------------------------------------- > > (Updated March 24, 2015, 10:15 p.m.) > > > Review request for mesos, Dominic Hamon, Ian Downes, and Jie Yu. > > > Bugs: mesos-2136 > https://issues.apache.org/jira/browse/mesos-2136 > > > Repository: mesos > > > Description > ------- > > Added cgroup memory pressure listening tests. > > > Diffs > ----- > > src/Makefile.am 7a06c7028eca8164b1f5fdea6a7ecd37ee6826bb > src/tests/cgroups_tests.cpp 75c61aad80f894acb92a9752e8d1b6af70e5b9a6 > src/tests/memory_test_helper.hpp PRE-CREATION > src/tests/memory_test_helper.cpp PRE-CREATION > src/tests/memory_test_helper_main.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/31276/diff/ > > > Testing > ------- > > > Thanks, > > Chi Zhang > >
