mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bernd Mathiske <be...@mesosphere.io>
Subject Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.
Date Tue, 08 Mar 2016 11:25:04 GMT


> On March 4, 2016, 6:45 a.m., Bernd Mathiske wrote:
> >
> 
> Joseph Wu wrote:
>     I also noticed a couple of these:
>     ```
>       MesosSchedulerDriver driver(
>         &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
>     ```
>     Now fixed (there were two spaces rather than four).
>     
>     ---
>     
>     Also went through and changed a couple of these:
>     ```
>       Future<SlaveReregisteredMessage> slaveReregisteredMessage =
>         FUTURE_PROTOBUF(
>             SlaveReregisteredMessage(), master.get()->pid, slave.get()->pid);
>     ```
>     To:
>     ```
>       Future<SlaveReregisteredMessage> slaveReregisteredMessage =
>         FUTURE_PROTOBUF(
>             SlaveReregisteredMessage(), 
>             master.get()->pid, 
>             slave.get()->pid);
>     ```

OK!


> On March 4, 2016, 6:45 a.m., Bernd Mathiske wrote:
> > src/tests/master_tests.cpp, line 1140
> > <https://reviews.apache.org/r/43615/diff/13/?file=1280079#file1280079line1140>
> >
> >     It's OK to continue with the first arg on the same line in such cases.
> >     
> >     Here and elsewhere.
> 
> Joseph Wu wrote:
>     I believe the preferred style is:
>     ```
>     EXPECT_EQ(
>         ...,
>         ...);
>     ```
>     Rather than:
>     ```
>     EXPECT_EQ(...
>               ...);
>     ```
>     (This is based on how we indented the MasterMaintenanceTests.)

AFAIK, it depends on how long the args are. If they are short, either way is fine. If they
are "too long" you are always right :-)


- Bernd


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43615/#review122072
-----------------------------------------------------------


On March 4, 2016, 4:14 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43615/
> -----------------------------------------------------------
> 
> (Updated March 4, 2016, 4:14 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 f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
>   src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
>   src/tests/health_check_tests.cpp 59ef31970af2d255abe169dfbc2e6e0314d29e9a 
>   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 36595772b34bcb8d37dbc74d247bdf4614f10150 
>   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 6e9e2e64f1666c2a81f5d859ee014ee14365b6b0 
> 
> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message