You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2017/01/12 13:29:59 UTC
Review Request 55459: Added task check validation test.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55459/
-----------------------------------------------------------
Review request for mesos, Gast�n Kleiman, haosdent huang, and Vinod Kone.
Bugs: MESOS-6906
https://issues.apache.org/jira/browse/MESOS-6906
Repository: mesos
Description
-------
See summary.
Diffs
-----
src/Makefile.am 333b45683a10eaac3b653e006511306d8054922c
src/checks/checker.cpp PRE-CREATION
src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5
src/tests/check_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/55459/diff/
Testing
-------
make check
Thanks,
Alexander Rukletsov
Re: Review Request 55459: Added task check validation test.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55459/#review161383
-----------------------------------------------------------
Bad patch!
Reviews applied: [55459, 55458, 55457, 55456, 55455, 55454, 55453]
Failed command: python support/apply-reviews.py -n -r 55453
Error:
Traceback (most recent call last):
File "support/apply-reviews.py", line 349, in <module>
reviewboard()
File "support/apply-reviews.py", line 328, in reviewboard
apply_review()
File "support/apply-reviews.py", line 121, in apply_review
fetch_patch()
File "support/apply-reviews.py", line 150, in fetch_patch
r = urllib2.urlopen(patch_url(), context=ssl_create_default_context())
File "support/apply-reviews.py", line 131, in ssl_create_default_context
context = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
AttributeError: 'module' object has no attribute 'SSLContext'
Error in atexit._run_exitfuncs:
Traceback (most recent call last):
File "/usr/lib/python2.7/atexit.py", line 24, in _run_exitfuncs
func(*targs, **kargs)
File "support/apply-reviews.py", line 119, in <lambda>
atexit.register(lambda: os.remove('%s.patch' % patch_id()))
OSError: [Errno 2] No such file or directory: '55453.patch'
Error in sys.exitfunc:
Traceback (most recent call last):
File "/usr/lib/python2.7/atexit.py", line 24, in _run_exitfuncs
func(*targs, **kargs)
File "support/apply-reviews.py", line 119, in <lambda>
atexit.register(lambda: os.remove('%s.patch' % patch_id()))
OSError: [Errno 2] No such file or directory: '55453.patch'
Full log: https://builds.apache.org/job/Mesos-Reviewbot/16695/console
- Mesos ReviewBot
On Jan. 12, 2017, 1:29 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55459/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2017, 1:29 p.m.)
>
>
> Review request for mesos, Gast�n Kleiman, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/Makefile.am 333b45683a10eaac3b653e006511306d8054922c
> src/checks/checker.cpp PRE-CREATION
> src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5
> src/tests/check_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55459/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 55459: Added task check validation test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Jan. 18, 2017, 11:38 a.m., Vinod Kone wrote:
> > src/tests/check_tests.cpp, line 114
> > <https://reviews.apache.org/r/55459/diff/2/?file=1607440#file1607440line114>
> >
> > Can you also add a test for valid cases?
They are embedded into this test: there are some `EXPECT_NONE`'s. I'll rename the test to reflect it.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55459/#review162073
-----------------------------------------------------------
On Jan. 18, 2017, 9:31 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55459/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2017, 9:31 a.m.)
>
>
> Review request for mesos, Gast�n Kleiman, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5
> src/tests/check_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55459/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 55459: Added task check validation test.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55459/#review162073
-----------------------------------------------------------
src/tests/check_tests.cpp (line 31)
<https://reviews.apache.org/r/55459/#comment233324>
s/CheckInfoProtobufValidation/Invalid/
src/tests/check_tests.cpp (line 37)
<https://reviews.apache.org/r/55459/#comment233321>
s/checkProto/check/
or
s/checkProto/checkInfo/
src/tests/check_tests.cpp (line 40)
<https://reviews.apache.org/r/55459/#comment233323>
I prefer actually doing an equality check on the actual error string, to make sure the validation is failing for the reason you are expecting. While doing an equality on string is brittle, I think it is worth it.
Here and below.
src/tests/check_tests.cpp (line 47)
<https://reviews.apache.org/r/55459/#comment233322>
// The associated message for a given type must be set.
src/tests/check_tests.cpp (line 114)
<https://reviews.apache.org/r/55459/#comment233325>
Can you also add a test for valid cases?
- Vinod Kone
On Jan. 18, 2017, 9:31 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55459/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2017, 9:31 a.m.)
>
>
> Review request for mesos, Gast�n Kleiman, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5
> src/tests/check_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55459/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 55459: Added task check validation test.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55459/#review162086
-----------------------------------------------------------
Bad patch!
Reviews applied: [55459, 55458, 55457, 55456, 55455, 55454, 55453]
Failed command: python support/apply-reviews.py -n -r 55458
Error:
2017-01-18 13:28:24 URL:https://reviews.apache.org/r/55458/diff/raw/ [7612/7612] -> "55458.patch" [1]
error: patch failed: src/master/validation.cpp:31
error: src/master/validation.cpp: patch does not apply
Full log: https://builds.apache.org/job/Mesos-Reviewbot/16766/console
- Mesos ReviewBot
On Jan. 18, 2017, 9:31 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55459/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2017, 9:31 a.m.)
>
>
> Review request for mesos, Gast�n Kleiman, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5
> src/tests/check_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55459/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 55459: Added task check validation test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Jan. 21, 2017, 3:39 p.m., Vinod Kone wrote:
> > Still planning to check against the actual strings?
I usually tend to avoid this (what you say in the comment), but we can add them.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55459/#review162561
-----------------------------------------------------------
On Jan. 20, 2017, 2:50 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55459/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2017, 2:50 p.m.)
>
>
> Review request for mesos, Gast�n Kleiman, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5
> src/tests/check_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55459/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 55459: Added task check validation test.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55459/#review162561
-----------------------------------------------------------
Ship it!
Still planning to check against the actual strings?
- Vinod Kone
On Jan. 20, 2017, 2:50 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55459/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2017, 2:50 p.m.)
>
>
> Review request for mesos, Gast�n Kleiman, haosdent huang, and Vinod Kone.
>
>
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5
> src/tests/check_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55459/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 55459: Added task check validation test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55459/
-----------------------------------------------------------
(Updated Jan. 20, 2017, 2:50 p.m.)
Review request for mesos, Gast�n Kleiman, haosdent huang, and Vinod Kone.
Changes
-------
Vinod's comments + extra test.
Bugs: MESOS-6906
https://issues.apache.org/jira/browse/MESOS-6906
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5
src/tests/check_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/55459/diff/
Testing
-------
make check
Thanks,
Alexander Rukletsov
Re: Review Request 55459: Added task check validation test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55459/
-----------------------------------------------------------
(Updated Jan. 18, 2017, 9:31 a.m.)
Review request for mesos, Gast�n Kleiman, haosdent huang, and Vinod Kone.
Changes
-------
Fixed a rebase mistake.
Bugs: MESOS-6906
https://issues.apache.org/jira/browse/MESOS-6906
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
src/tests/CMakeLists.txt b7adf4f3126611a57baf4f62b62b208bc0236da5
src/tests/check_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/55459/diff/
Testing
-------
make check
Thanks,
Alexander Rukletsov