----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65057/#review195745 -----------------------------------------------------------
src/tests/storage_local_resource_provider_tests.cpp Lines 2132 (patched) <https://reviews.apache.org/r/65057/#comment275032> Maybe "To accomplish this:" src/tests/storage_local_resource_provider_tests.cpp Lines 2134 (patched) <https://reviews.apache.org/r/65057/#comment275033> s/message// src/tests/storage_local_resource_provider_tests.cpp Lines 2172-2179 (patched) <https://reviews.apache.org/r/65057/#comment275034> Could you also add a note here regarding why the order of these two is reversed? Chun-Hung and I have such a comment in ours if you want to borrow it for consistency :) src/tests/storage_local_resource_provider_tests.cpp Lines 2189-2191 (patched) <https://reviews.apache.org/r/65057/#comment275035> Could you leave a more verbose comment here as to why this is necessary? Chun-Hung and my patches have one if you care to borrow for consistency :) src/tests/storage_local_resource_provider_tests.cpp Lines 2226 (patched) <https://reviews.apache.org/r/65057/#comment275036> Do we need this, or just being careful? If it is needed, it might not be if we add a long filter to the accept call. src/tests/storage_local_resource_provider_tests.cpp Lines 2278 (patched) <https://reviews.apache.org/r/65057/#comment275037> Benjamin suggested on my RR that we figure out the issue in `Slave::~Slave()` that creates the need for this resume. Looking at some other paused tests in the codebase, many of them do not need to resume. I agree that it's a good idea for us to fix this in 'cluster.cpp' and eliminate the need for this resume. - Greg Mann On Jan. 10, 2018, 12:25 a.m., Gaston Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65057/ > ----------------------------------------------------------- > > (Updated Jan. 10, 2018, 12:25 a.m.) > > > Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu. > > > Bugs: MESOS-8363 and MESOS-8420 > https://issues.apache.org/jira/browse/MESOS-8363 > https://issues.apache.org/jira/browse/MESOS-8420 > > > Repository: mesos > > > Description > ------- > > This patch adds > `StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate` that > verifies that operation status updates are resent to the master after > being dropped en route to it. > > > Diffs > ----- > > src/tests/storage_local_resource_provider_tests.cpp > bbfe95e9818f25fdd5405db3ad2fe355e023f743 > > > Diff: https://reviews.apache.org/r/65057/diff/4/ > > > Testing > ------- > > `sudo bin/mesos-tests.sh > --gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate' > --gtest_repeat=100 --gtest_break_on_failure` on GNU/Linux > > > Thanks, > > Gaston Kleiman > >
