Re: Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-28 Thread Chun-Hung Hsiao 

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

(Updated Nov. 29, 2018, 5:01 a.m.)


Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.


Changes
---

Fixed a test flake.


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


Repository: mesos


Description
---

Due to the changes of the `CREATE_DISK` semantics, this test is
rewritten to convert a preprovisioned volume to a profile volumes, and
then to destroy it to return the space back to the storage pool.

NOTE: The updated test will fail unless r/69361 (which implements the
new `CREATE_DISK` semantics) is also applied.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
077a46585bd56181ba199dc529e09f38f4971338 


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

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


Testing
---

make check

This test passes iff the next patch is applied.


Thanks,

Chun-Hung Hsiao

Re: Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-21 Thread Chun-Hung Hsiao 

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

(Updated Nov. 22, 2018, 6:31 a.m.)


Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.


Changes
---

Fixed a flake due to a race.


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


Repository: mesos


Description
---

Due to the changes of the `CREATE_DISK` semantics, this test is
rewritten to convert a preprovisioned volume to a profile volumes, and
then to destroy it to return the space back to the storage pool.

NOTE: The updated test will fail unless r/69361 (which implements the
new `CREATE_DISK` semantics) is also applied.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
077a46585bd56181ba199dc529e09f38f4971338 


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

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


Testing
---

make check

This test passes iff the next patch is applied.


Thanks,

Chun-Hung Hsiao

Re: Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-20 Thread Chun-Hung Hsiao 

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

(Updated Nov. 21, 2018, 2:46 a.m.)


Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.


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


Repository: mesos


Description (updated)
---

Due to the changes of the `CREATE_DISK` semantics, this test is
rewritten to convert a preprovisioned volume to a profile volumes, and
then to destroy it to return the space back to the storage pool.

NOTE: The updated test will fail unless r/69361 (which implements the
new `CREATE_DISK` semantics) is also applied.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
077a46585bd56181ba199dc529e09f38f4971338 


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


Testing
---

make check

This test passes iff the next patch is applied.


Thanks,

Chun-Hung Hsiao

Re: Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-20 Thread Benjamin Bannier 

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


Ship it!




Unless you reorder the chain, could you call out in the commit message the we 
will fix `StorageLocalResourceProviderTest.ImportPreprovisionedVolume` in a 
follow-up commit?

- Benjamin Bannier


On Nov. 19, 2018, 10:19 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69360/
> ---
> 
> (Updated Nov. 19, 2018, 10:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to the changes of the `CREATE_DISK` semantics, this test is
> rewritten to convert a preprovisioned volume to a profile volumes, and
> then to destroy it to return the space back to the storage pool.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69360/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes iff the next patch is applied.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-19 Thread Chun-Hung Hsiao 

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

(Updated Nov. 19, 2018, 9:19 p.m.)


Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.


Changes
---

Addressed James' and Benjamin's comments.


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


Repository: mesos


Description
---

Due to the changes of the `CREATE_DISK` semantics, this test is
rewritten to convert a preprovisioned volume to a profile volumes, and
then to destroy it to return the space back to the storage pool.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
077a46585bd56181ba199dc529e09f38f4971338 


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

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


Testing
---

make check

This test passes iff the next patch is applied.


Thanks,

Chun-Hung Hsiao

Re: Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-19 Thread Chun-Hung Hsiao 


> On Nov. 16, 2018, 12:02 p.m., Benjamin Bannier wrote:
> > This test does not pass for me, could you fix that?

This modified test won't pass without the next patch. However, if we switch the 
order, the unmodified test won't pass. So no I cannot fix this, unfortunately :(


- Chun-Hung


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


On Nov. 16, 2018, 12:02 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69360/
> ---
> 
> (Updated Nov. 16, 2018, 12:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to the changes of the `CREATE_DISK` semantics, this test is
> rewritten to convert a preprovisioned volume to a profile volumes, and
> then to destroy it to return the space back to the storage pool.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69360/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes iff the next patch is applied.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-16 Thread Benjamin Bannier 

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



This test does not pass for me, could you fix that?


src/tests/storage_local_resource_provider_tests.cpp
Line 2736 (original), 2723 (patched)


Since we assume the first offer contains the preprovisioned disks on the 
next line, we should also explicitly assert this here, e.g.,
```
ASSERT_EQ(1, rawDiskOffers->size());
```



src/tests/storage_local_resource_provider_tests.cpp
Line 2821 (original), 2761 (patched)


Ditto (assert exactly one offer).



src/tests/storage_local_resource_provider_tests.cpp
Line 2842 (original), 2786 (patched)


This could be `EXPECT_FALSE`.


- Benjamin Bannier


On Nov. 16, 2018, 1:02 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69360/
> ---
> 
> (Updated Nov. 16, 2018, 1:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to the changes of the `CREATE_DISK` semantics, this test is
> rewritten to convert a preprovisioned volume to a profile volumes, and
> then to destroy it to return the space back to the storage pool.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69360/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes iff the next patch is applied.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-15 Thread James DeFelice 

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




src/tests/storage_local_resource_provider_tests.cpp
Line 2655 (original), 2655 (patched)


s/into/as/ ?


- James DeFelice


On Nov. 16, 2018, 12:02 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69360/
> ---
> 
> (Updated Nov. 16, 2018, 12:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to the changes of the `CREATE_DISK` semantics, this test is
> rewritten to convert a preprovisioned volume to a profile volumes, and
> then to destroy it to return the space back to the storage pool.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69360/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes iff the next patch is applied.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-15 Thread James DeFelice 

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




src/tests/storage_local_resource_provider_tests.cpp
Line 2655 (original), 2655 (patched)


s/into/as/ ?


- James DeFelice


On Nov. 16, 2018, 12:02 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69360/
> ---
> 
> (Updated Nov. 16, 2018, 12:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to the changes of the `CREATE_DISK` semantics, this test is
> rewritten to convert a preprovisioned volume to a profile volumes, and
> then to destroy it to return the space back to the storage pool.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69360/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes iff the next patch is applied.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-15 Thread James DeFelice 

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




src/tests/storage_local_resource_provider_tests.cpp
Line 2655 (original), 2655 (patched)


s/into/as/ ?


- James DeFelice


On Nov. 16, 2018, 12:02 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69360/
> ---
> 
> (Updated Nov. 16, 2018, 12:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to the changes of the `CREATE_DISK` semantics, this test is
> rewritten to convert a preprovisioned volume to a profile volumes, and
> then to destroy it to return the space back to the storage pool.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 077a46585bd56181ba199dc529e09f38f4971338 
> 
> 
> Diff: https://reviews.apache.org/r/69360/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test passes iff the next patch is applied.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 69360: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

2018-11-15 Thread Chun-Hung Hsiao 

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

Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.


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


Repository: mesos


Description
---

Due to the changes of the `CREATE_DISK` semantics, this test is
rewritten to convert a preprovisioned volume to a profile volumes, and
then to destroy it to return the space back to the storage pool.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
077a46585bd56181ba199dc529e09f38f4971338 


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


Testing
---

make check

This test passes iff the next patch is applied.


Thanks,

Chun-Hung Hsiao