Re: Review Request 40732: Enabling ResourcesTest.Precision

2015-12-07 Thread Avinash sridharan 


> On Nov. 26, 2015, 3:35 p.m., Neil Conway wrote:
> > src/tests/resources_tests.cpp, line 1526
> > 
> >
> > 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

2015-12-07 Thread Avinash sridharan 


> On Nov. 26, 2015, 3:48 p.m., Klaus Ma wrote:
> > src/tests/resources_tests.cpp, line 1534
> > 
> >
> > 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

2015-11-26 Thread Mesos ReviewBot 

---
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

2015-11-26 Thread Neil Conway 

---
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)


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

2015-11-26 Thread Klaus Ma 

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



src/tests/resources_tests.cpp (line 1534)


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
> 
>

Review Request 40732: Enabling ResourcesTest.Precision

2015-11-25 Thread Avinash sridharan 

---
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