Re: Review Request 66060: Added end-to-end tests for operation feedback.

[Gaston Kleiman]

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

(Updated March 22, 2018, 12:06 p.m.)


Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Zhitao Li.


Changes
---

Fixed rebase.


Bugs: MESOS-8201
https://issues.apache.org/jira/browse/MESOS-8201


Repository: mesos


Description
---

Added end-to-end tests for operation feedback.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
0ad2d24c5b373016b46e319a287004fc6bdbc3af 


Diff: https://reviews.apache.org/r/66060/diff/5/

Changes: https://reviews.apache.org/r/66060/diff/4-5/


Testing
---

The tests passed over 1000 iterations on GNU/Linux.


Thanks,

Gaston Kleiman

Re: Review Request 66060: Added end-to-end tests for operation feedback.

[Gaston Kleiman]

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

(Updated March 22, 2018, 11:28 a.m.)


Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Zhitao Li.


Changes
---

Rebased.


Bugs: MESOS-8201
https://issues.apache.org/jira/browse/MESOS-8201


Repository: mesos


Description
---

Added end-to-end tests for operation feedback.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
0ad2d24c5b373016b46e319a287004fc6bdbc3af 


Diff: https://reviews.apache.org/r/66060/diff/4/

Changes: https://reviews.apache.org/r/66060/diff/3-4/


Testing
---

The tests passed over 1000 iterations on GNU/Linux.


Thanks,

Gaston Kleiman

Re: Review Request 66060: Added end-to-end tests for operation feedback.

[Gaston Kleiman]

> On March 20, 2018, 2:32 p.m., Greg Mann wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 3323 (patched)
> > 
> >
> > s/are/is/

Updated this patch, and also posted this RR fixing the same error in the rest 
of the file: https://reviews.apache.org/r/66182/


- Gaston


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


On March 20, 2018, 4:49 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66060/
> ---
> 
> (Updated March 20, 2018, 4:49 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Zhitao Li.
> 
> 
> Bugs: MESOS-8201
> https://issues.apache.org/jira/browse/MESOS-8201
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added end-to-end tests for operation feedback.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 264d42b57fe1a8ea04c1de0a10112878c7b45d39 
> 
> 
> Diff: https://reviews.apache.org/r/66060/diff/3/
> 
> 
> Testing
> ---
> 
> The tests passed over 1000 iterations on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66060: Added end-to-end tests for operation feedback.

[Gaston Kleiman]

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

(Updated March 20, 2018, 4:49 p.m.)


Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Zhitao Li.


Changes
---

Addressed Greg's feedback.


Bugs: MESOS-8201
https://issues.apache.org/jira/browse/MESOS-8201


Repository: mesos


Description
---

Added end-to-end tests for operation feedback.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
264d42b57fe1a8ea04c1de0a10112878c7b45d39 


Diff: https://reviews.apache.org/r/66060/diff/3/

Changes: https://reviews.apache.org/r/66060/diff/2-3/


Testing
---

The tests passed over 1000 iterations on GNU/Linux.


Thanks,

Gaston Kleiman

Re: Review Request 66060: Added end-to-end tests for operation feedback.

[Greg Mann]

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


Fix it, then Ship it!





src/tests/storage_local_resource_provider_tests.cpp
Lines 3323 (patched)


s/are/is/



src/tests/storage_local_resource_provider_tests.cpp
Lines 3409 (patched)


Should we do `ASSERT_TRUE(resource.has_provider.id());` here?



src/tests/storage_local_resource_provider_tests.cpp
Lines 3417-3418 (patched)


Nit: could you switch the order of these lines?


- Greg Mann


On March 19, 2018, 6:09 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66060/
> ---
> 
> (Updated March 19, 2018, 6:09 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Zhitao Li.
> 
> 
> Bugs: MESOS-8201
> https://issues.apache.org/jira/browse/MESOS-8201
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added end-to-end tests for operation feedback.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 264d42b57fe1a8ea04c1de0a10112878c7b45d39 
> 
> 
> Diff: https://reviews.apache.org/r/66060/diff/2/
> 
> 
> Testing
> ---
> 
> The tests passed over 1000 iterations on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66060: Added end-to-end tests for operation feedback.

[Gaston Kleiman]

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

(Updated March 19, 2018, 11:09 a.m.)


Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Zhitao Li.


Changes
---

Addressed Zhitao's feedback.

Removed unnecessary test.


Bugs: MESOS-8201
https://issues.apache.org/jira/browse/MESOS-8201


Repository: mesos


Description
---

Added end-to-end tests for operation feedback.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
264d42b57fe1a8ea04c1de0a10112878c7b45d39 


Diff: https://reviews.apache.org/r/66060/diff/2/

Changes: https://reviews.apache.org/r/66060/diff/1-2/


Testing
---

The tests passed over 1000 iterations on GNU/Linux.


Thanks,

Gaston Kleiman

Re: Review Request 66060: Added end-to-end tests for operation feedback.

[Gaston Kleiman]

> On March 19, 2018, 10:12 a.m., Zhitao Li wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 3282 (patched)
> > 
> >
> > Is there a reason this test requires `ROOT_` filter? A lot of people do 
> > not run such tests by default with `make check` so we could reduce coverage.

Yeah, I am afraid that the CSI plugin needs to run as root.


> On March 19, 2018, 10:12 a.m., Zhitao Li wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 3604 (patched)
> > 
> >
> > Where is this magic string `"operation"` come from? Can we define a 
> > local const?

This is just the name of the operation, it comes from line 3600. We could make 
it a local const.


- Gaston


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


On March 16, 2018, 12:58 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66060/
> ---
> 
> (Updated March 16, 2018, 12:58 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Zhitao Li.
> 
> 
> Bugs: MESOS-8201
> https://issues.apache.org/jira/browse/MESOS-8201
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added end-to-end tests for operation feedback.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 264d42b57fe1a8ea04c1de0a10112878c7b45d39 
> 
> 
> Diff: https://reviews.apache.org/r/66060/diff/1/
> 
> 
> Testing
> ---
> 
> The tests passed over 1000 iterations on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66060: Added end-to-end tests for operation feedback.

[Zhitao Li]

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




src/tests/storage_local_resource_provider_tests.cpp
Lines 3282 (patched)


Is there a reason this test requires `ROOT_` filter? A lot of people do not 
run such tests by default with `make check` so we could reduce coverage.



src/tests/storage_local_resource_provider_tests.cpp
Lines 3411 (patched)


empty line after `foreach` block



src/tests/storage_local_resource_provider_tests.cpp
Lines 3604 (patched)


Where is this magic string `"operation"` come from? Can we define a local 
const?


- Zhitao Li


On March 16, 2018, 12:58 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66060/
> ---
> 
> (Updated March 16, 2018, 12:58 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Zhitao Li.
> 
> 
> Bugs: MESOS-8201
> https://issues.apache.org/jira/browse/MESOS-8201
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added end-to-end tests for operation feedback.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 264d42b57fe1a8ea04c1de0a10112878c7b45d39 
> 
> 
> Diff: https://reviews.apache.org/r/66060/diff/1/
> 
> 
> Testing
> ---
> 
> The tests passed over 1000 iterations on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66060: Added end-to-end tests for operation feedback.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63991, 63992, 63994, 65300, 64618, 65993, 65994, 66056, 
66057, 66058, 66059, 66060]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On March 14, 2018, 9:29 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66060/
> ---
> 
> (Updated March 14, 2018, 9:29 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8201
> https://issues.apache.org/jira/browse/MESOS-8201
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added end-to-end tests for operation feedback.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 264d42b57fe1a8ea04c1de0a10112878c7b45d39 
> 
> 
> Diff: https://reviews.apache.org/r/66060/diff/1/
> 
> 
> Testing
> ---
> 
> The tests passed over 1000 iterations on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 66060: Added end-to-end tests for operation feedback.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66060 was successfully built and tested.

Reviews applied: `['63991', '63992', '63994', '65300', '64618', '65993', 
'65994', '66056', '66057', '66058', '66059', '66060']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66060

- Mesos Reviewbot Windows


On March 14, 2018, 10:29 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66060/
> ---
> 
> (Updated March 14, 2018, 10:29 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8201
> https://issues.apache.org/jira/browse/MESOS-8201
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added end-to-end tests for operation feedback.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 264d42b57fe1a8ea04c1de0a10112878c7b45d39 
> 
> 
> Diff: https://reviews.apache.org/r/66060/diff/1/
> 
> 
> Testing
> ---
> 
> The tests passed over 1000 iterations on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Review Request 66060: Added end-to-end tests for operation feedback.

[Gaston Kleiman]

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

Review request for mesos, Chun-Hung Hsiao and Greg Mann.


Bugs: MESOS-8201
https://issues.apache.org/jira/browse/MESOS-8201


Repository: mesos


Description
---

Added end-to-end tests for operation feedback.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
264d42b57fe1a8ea04c1de0a10112878c7b45d39 


Diff: https://reviews.apache.org/r/66060/diff/1/


Testing
---

The tests passed over 1000 iterations on GNU/Linux.


Thanks,

Gaston Kleiman