> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 138 (patched) > > <https://reviews.apache.org/r/56213/diff/3/?file=1627707#file1627707line138> > > > > no need for this.
For implicit reconciliation, `tasks` are empty, hence we need to ensure the `Call` message is properly formed. > On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 171 (patched) > > <https://reviews.apache.org/r/56213/diff/3/?file=1627707#file1627707line171> > > > > Instead of these long test names how about you create a fixture for > > CommandExecutor check tests for better namespacing? > > > > class CommandExecutorCheckTest : public CheckTest {}; > > > > TEST_F(CommandExecutorCheckTest, CommandCheckDeliveredAndReconciled) Good idea! > On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 177 (patched) > > <https://reviews.apache.org/r/56213/diff/3/?file=1627707#file1627707line177> > > > > s/agent/slave/ > > > > here and everywhere else. > > > > we are not doing this change yet. > > Gastón Kleiman wrote: > We do this in `health_check_tests.cpp`: > > ``` > $ grep "agent = " health_check_tests.cpp | wc -l > 15 > $ grep "slave = " health_check_tests.cpp | wc -l > 0 > $ > ``` > > Vinod Kone wrote: > I think we should fix `health_check_tests.cpp` as well then. > > Gastón Kleiman wrote: > I used `health_check_tests.cpp` just as an example, but we're using this > in a lot of places: > > ``` > $ git grep "agent[^ ]* =" | cut -d: -f1 | sort | uniq > api_tests.cpp > authentication_tests.cpp > containerizer/cgroups_isolator_tests.cpp > containerizer/docker_containerizer_tests.cpp > containerizer/io_switchboard_tests.cpp > containerizer/memory_pressure_tests.cpp > fault_tolerance_tests.cpp > health_check_tests.cpp > hierarchical_allocator_tests.cpp > hook_tests.cpp > master_allocator_tests.cpp > master_tests.cpp > master_validation_tests.cpp > mesos.cpp > mesos.hpp > metrics_tests.cpp > oversubscription_tests.cpp > partition_tests.cpp > reconciliation_tests.cpp > reservation_endpoints_tests.cpp > reservation_tests.cpp > slave_authorization_tests.cpp > slave_recovery_tests.cpp > slave_tests.cpp > sorter_tests.cpp > $ git grep "agent[^ ]* =" | wc -l > 207 > $ > ``` The policy I'm sticking to is to use `agent` in all new or being modified non-user facing code. I'd suggest we keep `agent` here and update in other places as we go. > On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 237 (patched) > > <https://reviews.apache.org/r/56213/diff/3/?file=1627707#file1627707line237> > > > > s/checkCommand/command/ > > > > here and below. We use `command` for task's command, e.g., in `HTTPCheckDelivered`, hence I'd prefer to keep `checkCommand` to disambiguate. > On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 391 (patched) > > <https://reviews.apache.org/r/56213/diff/3/?file=1627707#file1627707line391> > > > > s/checkCommandString/command/ To avoid the aforementioned ambiguity, I'd go with `checkCommand`. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review165049 ----------------------------------------------------------- On Feb. 28, 2017, 3:55 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56213/ > ----------------------------------------------------------- > > (Updated Feb. 28, 2017, 3:55 p.m.) > > > Review request for mesos, Gastón Kleiman and Vinod Kone. > > > Bugs: MESOS-6906 > https://issues.apache.org/jira/browse/MESOS-6906 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/tests/check_tests.cpp b5a5d8e80c80b480992a3c8160ee7d0e4443111c > src/tests/mesos.hpp b450a04dfbf3bbeeb6b605fb78097dca390cbdbe > > > Diff: https://reviews.apache.org/r/56213/diff/3/ > > > Testing > ------- > > See https://reviews.apache.org/r/56218/ > > > Thanks, > > Alexander Rukletsov > >
