> On March 8, 2016, 3:50 a.m., Bernd Mathiske wrote: > > src/tests/scheduler_event_call_tests.cpp, line 367 > > <https://reviews.apache.org/r/43615/diff/14/?file=1281494#file1281494line367> > > > > In most other places you have the blank line before the detector.
True. Fixed four places in: - scheduler_driver_tests.cpp - scheduler_event_call_tests.cpp - slave_tests.cpp > On March 8, 2016, 3:50 a.m., Bernd Mathiske wrote: > > src/tests/slave_tests.cpp, line 180 > > <https://reviews.apache.org/r/43615/diff/14/?file=1281496#file1281496line180> > > > > With this line you have now introduced a 3rd way to write this stretch > > of code. Please pick one. This one is probably the least controversial. :-) I see the following patterns: ``` Owned<MasterDetector> detector = master.get()->detector(); Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), ...); ASSERT_SOME(slave); ``` --- ``` // There may or may not be a comment here. i.e. "Start a slave." Owned<MasterDetector> detector = master.get()->detector(); Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), ...); ASSERT_SOME(slave); ``` --- ``` slave::Flags flags = CreateSlaveFlags(); Owned<MasterDetector> detector = master.get()->detector(); Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), ...); ASSERT_SOME(slave); ``` I'll standardize to: ``` // Maybe: slave::Flags flags = CreateSlaveFlags(); Owned<MasterDetector> detector = master.get()->detector(); // Maybe a comment here. Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), ...); ASSERT_SOME(slave); ``` - Joseph ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43615/#review122502 ----------------------------------------------------------- On March 8, 2016, 2:33 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43615/ > ----------------------------------------------------------- > > (Updated March 8, 2016, 2:33 p.m.) > > > Review request for mesos, Bernd Mathiske and Artem Harutyunyan. > > > Bugs: MESOS-4633 and MESOS-4634 > https://issues.apache.org/jira/browse/MESOS-4633 > https://issues.apache.org/jira/browse/MESOS-4634 > > > Repository: mesos > > > Description > ------- > > Includes the following changes: > > * Added the `<process/owned.hpp>` header where appropriate. > * Added the namespace `using process::Owned;` where appropriate. > * Generally replaced `Try<PID<Master>>` with `Owned<cluster::Master>`. And > `Try<PID<Slave>>` with `Owned<cluster::Slave>`. > * Added the (now required) `MasterDetector` argument to all slaves. Before, > this was fetched from the first master in `Cluster`. > * Removed `Shutdown();` from all tests. > * Replaced `Stop(...)` with the appropriate master/slave destruction calls. > * Wrap various slave objects in `Owned` (i.e. containerizers, isolators, > launchers, etc). > * Replace `CHECK` in tests with `ASSERT`. > > > Diffs > ----- > > src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 > src/tests/command_executor_tests.cpp > 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 > src/tests/container_logger_tests.cpp > 00f4129e46aa9268fbb66da25b34e61004fa87b2 > src/tests/containerizer/docker_containerizer_tests.cpp > 6aecd912fc84b72d2b64f7a842891fddcbc469ac > src/tests/containerizer/external_containerizer_test.cpp > 8e1dbe306a088eb16cd3b9c6174b95fad5685da4 > src/tests/containerizer/filesystem_isolator_tests.cpp > e72239a55724f1aeeec5362cc370c93dbeca7164 > src/tests/containerizer/isolator_tests.cpp > 342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 > src/tests/containerizer/memory_pressure_tests.cpp > 03879d99c371f296f8d9904666911b34209c114d > src/tests/containerizer/mesos_containerizer_tests.cpp > 15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 > src/tests/containerizer/port_mapping_tests.cpp > 983a63333be160aefe5a32acb6111bb3c85230ec > src/tests/containerizer/provisioner_docker_tests.cpp > 5b685bfd842d0d98e8ea5ec5ddea8d2cd893dd81 > src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 > src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 > src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b > src/tests/executor_http_api_tests.cpp > 2fc0893f5f5e80a783296fb31b30abe86d92df1b > src/tests/fault_tolerance_tests.cpp > d193897e636efd0e3ef67bf67fcd6255a3de0341 > src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf > src/tests/health_check_tests.cpp d0fd27fd8a6b48511ef8cafab5dff59f65729d9f > src/tests/hook_tests.cpp 88ad7806bf9f3194e434743c35199a896edea92c > src/tests/master_allocator_tests.cpp > cba7c36471f93b678d94e1da0251a28a893696b1 > src/tests/master_authorization_tests.cpp > 29c89fb11da792c3e71eb880a19657ea225b3cc8 > src/tests/master_contender_detector_tests.cpp > 255ab8119a04b55bb4f1b61dee19c4be64499376 > src/tests/master_quota_tests.cpp 4fabc1473ec3e048afe7171abbb8d6e49e863847 > src/tests/master_slave_reconciliation_tests.cpp > d41178eb41df519073fc0890c5716bbc9fed6ad2 > src/tests/master_tests.cpp 2f4d820e223a48700ce1ac3a91b7256cc836c268 > src/tests/master_validation_tests.cpp > c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 > src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd > src/tests/monitor_tests.cpp 869c9e032817e8859a968232d4a61556a3d53d45 > src/tests/oversubscription_tests.cpp > e528476cd83b0e3f7ae8cea7d86dfabc1f66484e > src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 > src/tests/persistent_volume_endpoints_tests.cpp > 81185a161498394020a27f1f5bf747bac5425f43 > src/tests/persistent_volume_tests.cpp > bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 > src/tests/rate_limiting_tests.cpp 352a5e7a58b500c25c7c8a421047c4e79434e38e > src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 > src/tests/registrar_zookeeper_tests.cpp > 3df9779ee5d076e16f6a538326693a36f986b6d0 > src/tests/repair_tests.cpp bb104562659e135492f9857e5b452c8a0a9e97da > src/tests/reservation_endpoints_tests.cpp > f95ae7a32c3809d150adf1e9e515a3b527e61699 > src/tests/reservation_tests.cpp d7f9de6f2bce061316916260f356efdb96ecd482 > src/tests/role_tests.cpp fc3a72894631279460ee7971a4627d73c3d8c351 > src/tests/scheduler_driver_tests.cpp > f6dc25d82ae5f1e77fc6ede7ff2660ed0d9ea039 > src/tests/scheduler_event_call_tests.cpp > 8c02ceeb3ec1783cb2f63f100700508e70f586e4 > src/tests/scheduler_http_api_tests.cpp > dfb0f51fec67a3951e396eab28eedb0dbf9493ae > src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 > src/tests/status_update_manager_tests.cpp > d64d3b8c96270478f6b681c038de77c3a9eb68fe > src/tests/teardown_tests.cpp 2b8e6875022910a729732cee5e03f03099fe08e7 > > Diff: https://reviews.apache.org/r/43615/diff/ > > > Testing > ------- > > sudo make check > ``` > | OSX | CentOS 7 | CentOS 6 | Debian 8 | Ubuntu 12 | Ubuntu 14 | > Ubuntu 15 | > Non-SSL | :) | !@#$ | !^ | ^& | :) | :) | > :) | > With-SSL | :) | !@ $^&* | ! | :) | ^ | :) | > ^ | > > :) = Passed. > ! = DockerContainerizerTest.ROOT_DOCKER_LaunchWithPersistentVolumes > @ = ProvisionerDockerRegistryPullerTest.ROOT_INTERNET_CURL_ShellCommand > # = LinuxFilesystemIsolatorTest.ROOT_VolumeFromSandbox > $ = LinuxFilesystemIsolatorTest.ROOT_RecoverOrphanedPersistentVolume > % = MemoryPressureMesosTest.CGROUPS_ROOT_Statistics > ^ = MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery > & = LinuxFilesystemIsolatorTest.ROOT_SandboxEnvironmentVariable > * = LinuxFilesystemIsolatorTest.ROOT_MultipleContainers > ``` > > > Thanks, > > Joseph Wu > >
