You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@apache.org> on 2018/11/15 23:58:37 UTC
Review Request 69357: Added profiles to storage pools in tests for
`CREATE_DISK`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69357/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, Greg Mann, and Jan Schlicht.
Bugs: MESOS-9275
https://issues.apache.org/jira/browse/MESOS-9275
Repository: mesos
Description
-------
This patch adds a new `targetProfile` parameter to the `CREATE_DISK`
test helper, and add profiles to all storage pools in tests.
Tests relying on preprovisioned volumes are fixed in subsequent patches.
Diffs
-----
src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133
src/tests/master_tests.cpp ac6bf379c5906cf9612284911c121c9457f648a0
src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e
src/tests/operation_reconciliation_tests.cpp 37d38b3df8c162bd1baa5ce557e54baa5c23a006
src/tests/resource_provider_manager_tests.cpp 5bb740edc7c3cc8698aede4f2ed57c21232fe378
src/tests/storage_local_resource_provider_tests.cpp 077a46585bd56181ba199dc529e09f38f4971338
Diff: https://reviews.apache.org/r/69357/diff/1/
Testing
-------
`make check`
Tests `ReconcileDroppedOperation` and `ConvertPreExistingVolume` are fixed later in chain.
Thanks,
Chun-Hung Hsiao
Re: Review Request 69357: Added profiles to storage pools in tests for
`CREATE_DISK`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On Nov. 16, 2018, 12:01 p.m., Benjamin Bannier wrote:
> > Can you make sure that this patch does not produce any failing tests (might be triggered by earlier patch)?
Done for most patches, except for r/69360 and r/69361.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69357/#review210603
-----------------------------------------------------------
On Nov. 15, 2018, 11:58 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69357/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2018, 11:58 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jan Schlicht.
>
>
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new `targetProfile` parameter to the `CREATE_DISK`
> test helper, and add profiles to all storage pools in tests.
>
> Tests relying on preprovisioned volumes are fixed in subsequent patches.
>
>
> Diffs
> -----
>
> src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133
> src/tests/master_tests.cpp ac6bf379c5906cf9612284911c121c9457f648a0
> src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e
> src/tests/operation_reconciliation_tests.cpp 37d38b3df8c162bd1baa5ce557e54baa5c23a006
> src/tests/resource_provider_manager_tests.cpp 5bb740edc7c3cc8698aede4f2ed57c21232fe378
> src/tests/storage_local_resource_provider_tests.cpp 077a46585bd56181ba199dc529e09f38f4971338
>
>
> Diff: https://reviews.apache.org/r/69357/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
> Tests `ReconcileDroppedOperation` and `ConvertPreExistingVolume` are fixed later in chain.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 69357: Added profiles to storage pools in tests for
`CREATE_DISK`.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69357/#review210603
-----------------------------------------------------------
Can you make sure that this patch does not produce any failing tests (might be triggered by earlier patch)?
src/tests/master_tests.cpp
Lines 8870-8875 (original), 8870-8875 (patched)
<https://reviews.apache.org/r/69357/#comment295311>
nit: fits on a single line.
src/tests/storage_local_resource_provider_tests.cpp
Lines 2974-2975 (original), 2974 (patched)
<https://reviews.apache.org/r/69357/#comment295312>
Unrelated to this patch?
src/tests/storage_local_resource_provider_tests.cpp
Lines 3129-3130 (original), 3128 (patched)
<https://reviews.apache.org/r/69357/#comment295313>
Ditto.
- Benjamin Bannier
On Nov. 16, 2018, 12:58 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69357/
> -----------------------------------------------------------
>
> (Updated Nov. 16, 2018, 12:58 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jan Schlicht.
>
>
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new `targetProfile` parameter to the `CREATE_DISK`
> test helper, and add profiles to all storage pools in tests.
>
> Tests relying on preprovisioned volumes are fixed in subsequent patches.
>
>
> Diffs
> -----
>
> src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133
> src/tests/master_tests.cpp ac6bf379c5906cf9612284911c121c9457f648a0
> src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e
> src/tests/operation_reconciliation_tests.cpp 37d38b3df8c162bd1baa5ce557e54baa5c23a006
> src/tests/resource_provider_manager_tests.cpp 5bb740edc7c3cc8698aede4f2ed57c21232fe378
> src/tests/storage_local_resource_provider_tests.cpp 077a46585bd56181ba199dc529e09f38f4971338
>
>
> Diff: https://reviews.apache.org/r/69357/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
> Tests `ReconcileDroppedOperation` and `ConvertPreExistingVolume` are fixed later in chain.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 69357: Added profiles to storage pools in tests for
`CREATE_DISK`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On Nov. 20, 2018, 3:47 p.m., Benjamin Bannier wrote:
> > src/tests/operation_reconciliation_tests.cpp
> > Lines 795-797 (original), 795-798 (patched)
> > <https://reviews.apache.org/r/69357/diff/2/?file=2108786#file2108786line795>
> >
> > Nit: old indent was fine IMO.
Dropping this and created https://issues.apache.org/jira/browse/MESOS-9402.
> On Nov. 20, 2018, 3:47 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Line 4137 (original), 4135-4138 (patched)
> > <https://reviews.apache.org/r/69357/diff/2/?file=2108788#file2108788line4137>
> >
> > Nit: overindented, see change below.
Dropping this and created https://issues.apache.org/jira/browse/MESOS-9402.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69357/#review210719
-----------------------------------------------------------
On Nov. 19, 2018, 9:09 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69357/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2018, 9:09 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jan Schlicht.
>
>
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new `targetProfile` parameter to the `CREATE_DISK`
> test helper, and add profiles to all storage pools in tests.
>
> Tests relying on preprovisioned volumes are fixed in subsequent patches.
>
>
> Diffs
> -----
>
> src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133
> src/tests/master_tests.cpp ac6bf379c5906cf9612284911c121c9457f648a0
> src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e
> src/tests/operation_reconciliation_tests.cpp 37d38b3df8c162bd1baa5ce557e54baa5c23a006
> src/tests/resource_provider_manager_tests.cpp 5bb740edc7c3cc8698aede4f2ed57c21232fe378
> src/tests/storage_local_resource_provider_tests.cpp 077a46585bd56181ba199dc529e09f38f4971338
>
>
> Diff: https://reviews.apache.org/r/69357/diff/2/
>
>
> Testing
> -------
>
> `make check`
>
> Tests `ReconcileDroppedOperation` and `ConvertPreExistingVolume` are fixed later in chain.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 69357: Added profiles to storage pools in tests for
`CREATE_DISK`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On Nov. 20, 2018, 3:47 p.m., Benjamin Bannier wrote:
> > Could you update the commit message?
> >
> > I am not sure we need to update all the tests you touched here, but there also seems nothing wrong with it. Maybe something to potentially elaborate on in the commit message as well.
We need to adhere to the requirement of `target_profile`. I'll update the message.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69357/#review210719
-----------------------------------------------------------
On Nov. 19, 2018, 9:09 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69357/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2018, 9:09 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jan Schlicht.
>
>
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new `targetProfile` parameter to the `CREATE_DISK`
> test helper, and add profiles to all storage pools in tests.
>
> Tests relying on preprovisioned volumes are fixed in subsequent patches.
>
>
> Diffs
> -----
>
> src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133
> src/tests/master_tests.cpp ac6bf379c5906cf9612284911c121c9457f648a0
> src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e
> src/tests/operation_reconciliation_tests.cpp 37d38b3df8c162bd1baa5ce557e54baa5c23a006
> src/tests/resource_provider_manager_tests.cpp 5bb740edc7c3cc8698aede4f2ed57c21232fe378
> src/tests/storage_local_resource_provider_tests.cpp 077a46585bd56181ba199dc529e09f38f4971338
>
>
> Diff: https://reviews.apache.org/r/69357/diff/2/
>
>
> Testing
> -------
>
> `make check`
>
> Tests `ReconcileDroppedOperation` and `ConvertPreExistingVolume` are fixed later in chain.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 69357: Added profiles to storage pools in tests for
`CREATE_DISK`.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69357/#review210719
-----------------------------------------------------------
Fix it, then Ship it!
Could you update the commit message?
I am not sure we need to update all the tests you touched here, but there also seems nothing wrong with it. Maybe something to potentially elaborate on in the commit message as well.
src/tests/operation_reconciliation_tests.cpp
Lines 795-797 (original), 795-798 (patched)
<https://reviews.apache.org/r/69357/#comment295451>
Nit: old indent was fine IMO.
src/tests/storage_local_resource_provider_tests.cpp
Line 4137 (original), 4135-4138 (patched)
<https://reviews.apache.org/r/69357/#comment295452>
Nit: overindented, see change below.
- Benjamin Bannier
On Nov. 19, 2018, 10:09 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69357/
> -----------------------------------------------------------
>
> (Updated Nov. 19, 2018, 10:09 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Greg Mann, and Jan Schlicht.
>
>
> Bugs: MESOS-9275
> https://issues.apache.org/jira/browse/MESOS-9275
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new `targetProfile` parameter to the `CREATE_DISK`
> test helper, and add profiles to all storage pools in tests.
>
> Tests relying on preprovisioned volumes are fixed in subsequent patches.
>
>
> Diffs
> -----
>
> src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133
> src/tests/master_tests.cpp ac6bf379c5906cf9612284911c121c9457f648a0
> src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e
> src/tests/operation_reconciliation_tests.cpp 37d38b3df8c162bd1baa5ce557e54baa5c23a006
> src/tests/resource_provider_manager_tests.cpp 5bb740edc7c3cc8698aede4f2ed57c21232fe378
> src/tests/storage_local_resource_provider_tests.cpp 077a46585bd56181ba199dc529e09f38f4971338
>
>
> Diff: https://reviews.apache.org/r/69357/diff/2/
>
>
> Testing
> -------
>
> `make check`
>
> Tests `ReconcileDroppedOperation` and `ConvertPreExistingVolume` are fixed later in chain.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 69357: Added profiles to storage pools in tests for
`CREATE_DISK`.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69357/
-----------------------------------------------------------
(Updated Nov. 19, 2018, 9:09 p.m.)
Review request for mesos, Benjamin Bannier, Greg Mann, and Jan Schlicht.
Changes
-------
Addressed Benjamin's comments.
Bugs: MESOS-9275
https://issues.apache.org/jira/browse/MESOS-9275
Repository: mesos
Description
-------
This patch adds a new `targetProfile` parameter to the `CREATE_DISK`
test helper, and add profiles to all storage pools in tests.
Tests relying on preprovisioned volumes are fixed in subsequent patches.
Diffs (updated)
-----
src/tests/api_tests.cpp fdd9f871f75617fc26a28679e2a1e41f506c6133
src/tests/master_tests.cpp ac6bf379c5906cf9612284911c121c9457f648a0
src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e
src/tests/operation_reconciliation_tests.cpp 37d38b3df8c162bd1baa5ce557e54baa5c23a006
src/tests/resource_provider_manager_tests.cpp 5bb740edc7c3cc8698aede4f2ed57c21232fe378
src/tests/storage_local_resource_provider_tests.cpp 077a46585bd56181ba199dc529e09f38f4971338
Diff: https://reviews.apache.org/r/69357/diff/2/
Changes: https://reviews.apache.org/r/69357/diff/1-2/
Testing
-------
`make check`
Tests `ReconcileDroppedOperation` and `ConvertPreExistingVolume` are fixed later in chain.
Thanks,
Chun-Hung Hsiao