Bug 94502 – [Chromium] CCMathUtilTest.smallestAngleBetweenVectors unit test failing

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Kenneth Russell]

See for example:

http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Mac10.6/builds/21410/steps/webkit_unit_tests

Please fix ASAP in order to get the bots green.

[Shawn Singh]

looks like this util function and the unit test were added with webkit rev 126018;   +skyostil

In the meantime, I'm trying to repro the failure locally if possible.

[Kenneth Russell]

I'm going to disable this test for the moment until it's diagnosed.

[Kenneth Russell]

Committed r126049: <http://trac.webkit.org/changeset/126049>

[Shawn Singh]

(In reply to comment #3)
> Committed r126049: <http://trac.webkit.org/changeset/126049>

Just checking - was the additional TestExpectations stuff in this CL related to this unit test?

[Shawn Singh]

Created attachment 159496 [details]
Patch

please note that the ".0" suffix seemed to be necessary here, other wise std::min could not be resolved correctly

[Kenneth Russell]

Comment on attachment 159496 [details]
Patch

Per our offline discussion: are you sure that using higher precision for the intermediate results is really the right solution? No matter how many bits of precision are added, floating-point computations are inaccurate. Should the test be updated instead to allow for a small amount of inaccuracy?

BTW, the TestExpectations updates in the other commit were unrelated to this test.

[Shawn Singh]

Created attachment 159503 [details]
Patch

Thanks, good point =)  new patch is even more trivial, and we should follow-up asking Sami whether it's OK that this edge case is tested with less precision.

[Kenneth Russell]

Comment on attachment 159503 [details]
Patch

Looks good. r=me

[WebKit Review Bot]

Comment on attachment 159503 [details]
Patch

Clearing flags on attachment: 159503

Committed r126075: <http://trac.webkit.org/changeset/126075>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Sami Kyöstilä]

Thanks for fixing this guys. The landed patch looks fine. I didn't see the failure on my local machine or any Android phones, so I was wrongly assuming that EXPECT_EQ automatically uses an epsilon value for floats.