> On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: > > src/tests/oversubscription_tests.cpp, line 256 > > <https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line256> > > > > Why not await ready? Here and below :)
hmm because when we receive status 'running' from task, then we are pretty sure that Resource Estimator/QoS Controller is initialized (: or rather.. we want to make sure (: > On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: > > src/tests/mesos.hpp, lines 188-201 > > <https://reviews.apache.org/r/35157/diff/3/?file=980316#file980316line188> > > > > The StartSlave overload is growing a bit out of hand. Let's create a > > JIRA so we get those consolidated somehow. Can you add the ticket as a > > comment? :) MESOS-2833 > On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: > > src/tests/oversubscription_tests.cpp, line 209 > > <https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line209> > > > > Can you inline this below? (Is it used other places?) If not, should we > > const it? Here and below Added inline if possible, otherwise const. Added const also for the other variables which are not being changed during test. > On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: > > src/tests/oversubscription_tests.cpp, lines 234-238 > > <https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line234> > > > > Have you looked into, whether you can use createTask() instead? > > > > Here and below Good idea - mimized from e.g master_test. > On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: > > src/tests/oversubscription_tests.cpp, line 550 > > <https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line550> > > > > We tend to use the shorter form, if there is no ambguity (controller > > instead of 'qoSController' :) I'll leave that up to you, if you want to > > change it +1 Done. > On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: > > src/tests/oversubscription_tests.cpp, line 552 > > <https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line552> > > > > Let's introduce a 'using std::list;' above :) Actually - it was (: But i did not realize that, thx. > On June 8, 2015, 10:38 p.m., Niklas Nielsen wrote: > > src/tests/oversubscription_tests.cpp, line 618 > > <https://reviews.apache.org/r/35157/diff/3/?file=980318#file980318line618> > > > > Let's get the operator overload wired up and do ASSERT_EQ() :) Done - in type_utils - Bartek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35157/#review87074 ----------------------------------------------------------- On June 9, 2015, 12:17 a.m., Bartek Plotka wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35157/ > ----------------------------------------------------------- > > (Updated June 9, 2015, 12:17 a.m.) > > > Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Szymon Konefal, > and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > Added tests for QoS Controller and ResourceEstimator fetching ResoruceUsage > from ResourceMonitor. > > This is the unit test for https://reviews.apache.org/r/34980/ and > https://reviews.apache.org/r/35164/ > > > Diffs > ----- > > include/mesos/type_utils.hpp 52380c2461841026ee492797b4d8081f944f7b7b > src/common/type_utils.cpp e92f6f36de0955784619029a016667b46bbe221b > src/tests/mesos.hpp 087953d6bc716f11c315a0736f06f712d7f69417 > src/tests/mesos.cpp dff45b0d3bf9ef53f19575ab3d90a0b223755d6a > src/tests/oversubscription_tests.cpp > afd7ff4f2b50cb20cc2c8865b655ad1f8eb0c8b7 > > Diff: https://reviews.apache.org/r/35157/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Bartek Plotka > >
