You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Avinash sridharan <av...@mesosphere.io> on 2015/11/26 07:52:43 UTC
Review Request 40732: Enabling ResourcesTest.Precision
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40732/
-----------------------------------------------------------
Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.
Bugs: MESOS-3552
https://issues.apache.org/jira/browse/MESOS-3552
Repository: mesos
Description
-------
This is an existing test case to check precision of resource reservations, but would have always failed due to double precision errors. Forcing the test case to use EXPECT_DOUBE_EQ instead of EXPECT_EQ (strict equality)
Diffs
-----
src/tests/resources_tests.cpp dbd39cd5a6786682a7b528b6fea37ab78904cf12
Diff: https://reviews.apache.org/r/40732/diff/
Testing
-------
Ran make check against 40730
Thanks,
Avinash sridharan
Re: Review Request 40732: Enabling ResourcesTest.Precision
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40732/#review108097
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40730, 40732]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Nov. 26, 2015, 6:52 a.m., Avinash sridharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40732/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2015, 6:52 a.m.)
>
>
> Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.
>
>
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is an existing test case to check precision of resource reservations, but would have always failed due to double precision errors. Forcing the test case to use EXPECT_DOUBE_EQ instead of EXPECT_EQ (strict equality)
>
>
> Diffs
> -----
>
> src/tests/resources_tests.cpp dbd39cd5a6786682a7b528b6fea37ab78904cf12
>
> Diff: https://reviews.apache.org/r/40732/diff/
>
>
> Testing
> -------
>
> Ran make check against 40730
>
>
> Thanks,
>
> Avinash sridharan
>
>
Re: Review Request 40732: Enabling ResourcesTest.Precision
Posted by Avinash sridharan <av...@mesosphere.io>.
> On Nov. 26, 2015, 3:35 p.m., Neil Conway wrote:
> > src/tests/resources_tests.cpp, line 1526
> > <https://reviews.apache.org/r/40732/diff/1/?file=1147238#file1147238line1526>
> >
> > If we're going to enable this test, the comment should be removed.
Based on Klaus Ma's comments will either drop this commit, or if he agrees will remove this comment and keep it enabled.
- Avinash
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40732/#review108141
-----------------------------------------------------------
On Nov. 26, 2015, 6:52 a.m., Avinash sridharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40732/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2015, 6:52 a.m.)
>
>
> Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.
>
>
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is an existing test case to check precision of resource reservations, but would have always failed due to double precision errors. Forcing the test case to use EXPECT_DOUBE_EQ instead of EXPECT_EQ (strict equality)
>
>
> Diffs
> -----
>
> src/tests/resources_tests.cpp dbd39cd5a6786682a7b528b6fea37ab78904cf12
>
> Diff: https://reviews.apache.org/r/40732/diff/
>
>
> Testing
> -------
>
> Ran make check against 40730
>
>
> Thanks,
>
> Avinash sridharan
>
>
Re: Review Request 40732: Enabling ResourcesTest.Precision
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40732/#review108141
-----------------------------------------------------------
I don't think we're ready to enable this test yet. The test is checking that `operator==` for `Resources` tolerates floating-point roundoff error -- but it doesn't, yet. (Changing the test to use CHECK_DOUBLE_EQ() changes what is being tested.)
src/tests/resources_tests.cpp (line 1526)
<https://reviews.apache.org/r/40732/#comment167493>
If we're going to enable this test, the comment should be removed.
- Neil Conway
On Nov. 26, 2015, 6:52 a.m., Avinash sridharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40732/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2015, 6:52 a.m.)
>
>
> Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.
>
>
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is an existing test case to check precision of resource reservations, but would have always failed due to double precision errors. Forcing the test case to use EXPECT_DOUBE_EQ instead of EXPECT_EQ (strict equality)
>
>
> Diffs
> -----
>
> src/tests/resources_tests.cpp dbd39cd5a6786682a7b528b6fea37ab78904cf12
>
> Diff: https://reviews.apache.org/r/40732/diff/
>
>
> Testing
> -------
>
> Ran make check against 40730
>
>
> Thanks,
>
> Avinash sridharan
>
>
Re: Review Request 40732: Enabling ResourcesTest.Precision
Posted by Avinash sridharan <av...@mesosphere.io>.
> On Nov. 26, 2015, 3:48 p.m., Klaus Ma wrote:
> > src/tests/resources_tests.cpp, line 1534
> > <https://reviews.apache.org/r/40732/diff/1/?file=1147238#file1147238line1534>
> >
> > We can not change this to `EXPECT_DOUBLE_EQ` because it's used to check `operator==` in `Resources`. I think we can check the source code of `CHECK_NEAR` and re-use it in `Scalar::operator==`.
Maybe I am getting this wrong but in TEST(ResourcesTest, Precision) I thought we are explicitly checking cpu resources which are set to double? Hence the change to EXPECT_DOUBLE_EQ
- Avinash
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40732/#review108150
-----------------------------------------------------------
On Nov. 26, 2015, 6:52 a.m., Avinash sridharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40732/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2015, 6:52 a.m.)
>
>
> Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.
>
>
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is an existing test case to check precision of resource reservations, but would have always failed due to double precision errors. Forcing the test case to use EXPECT_DOUBE_EQ instead of EXPECT_EQ (strict equality)
>
>
> Diffs
> -----
>
> src/tests/resources_tests.cpp dbd39cd5a6786682a7b528b6fea37ab78904cf12
>
> Diff: https://reviews.apache.org/r/40732/diff/
>
>
> Testing
> -------
>
> Ran make check against 40730
>
>
> Thanks,
>
> Avinash sridharan
>
>
Re: Review Request 40732: Enabling ResourcesTest.Precision
Posted by Klaus Ma <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40732/#review108150
-----------------------------------------------------------
src/tests/resources_tests.cpp (line 1534)
<https://reviews.apache.org/r/40732/#comment167498>
We can not change this to `EXPECT_DOUBLE_EQ` because it's used to check `operator==` in `Resources`. I think we can check the source code of `CHECK_NEAR` and re-use it in `Scalar::operator==`.
- Klaus Ma
On Nov. 26, 2015, 2:52 p.m., Avinash sridharan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40732/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2015, 2:52 p.m.)
>
>
> Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.
>
>
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This is an existing test case to check precision of resource reservations, but would have always failed due to double precision errors. Forcing the test case to use EXPECT_DOUBE_EQ instead of EXPECT_EQ (strict equality)
>
>
> Diffs
> -----
>
> src/tests/resources_tests.cpp dbd39cd5a6786682a7b528b6fea37ab78904cf12
>
> Diff: https://reviews.apache.org/r/40732/diff/
>
>
> Testing
> -------
>
> Ran make check against 40730
>
>
> Thanks,
>
> Avinash sridharan
>
>