Bug 105060 – Add some unit testing for WTF::clampTo* functions

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Julien Chaffraix]

While investigating bug 83848, I discovered that we don't cover the clampTo functions which are used for the saturated arithmetic logic.

We do have some indirect testing as we have some saturated arithmetic layout tests (and some unit testing after 104955) but we may as well cover the functions themselves, at least for some basic coverage.

[Julien Chaffraix]

Created attachment 179548 [details]
Proposed testing, feedbacks seeked on whether more testing is possible accurately.

[Emil A Eklund]

Comment on attachment 179548 [details]
Proposed testing, feedbacks seeked on whether more testing is possible accurately.

View in context: https://bugs.webkit.org/attachment.cgi?id=179548&action=review

> Tools/TestWebKitAPI/Tests/WTF/MathExtras.cpp:53
> +TEST(WTF, clampToIntLong)

This test is identical to the long long one. Using long long should work on all platforms while long won't work work on 32 bit platforms or win x64 as maxInt + 1 will overflow a long on those platforms.

> Tools/TestWebKitAPI/Tests/WTF/MathExtras.cpp:157
> +    unsigned long overflowUnsigned = maxUnsigned + 1;

This will overflow on certain platforms (where sizeof int == sizeof long)

[Build Bot]

Comment on attachment 179548 [details]
Proposed testing, feedbacks seeked on whether more testing is possible accurately.

Attachment 179548 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15315857

[Julien Chaffraix]

Comment on attachment 179548 [details]
Proposed testing, feedbacks seeked on whether more testing is possible accurately.

View in context: https://bugs.webkit.org/attachment.cgi?id=179548&action=review

>> Tools/TestWebKitAPI/Tests/WTF/MathExtras.cpp:53
>> +TEST(WTF, clampToIntLong)
> 
> This test is identical to the long long one. Using long long should work on all platforms while long won't work work on 32 bit platforms or win x64 as maxInt + 1 will overflow a long on those platforms.

Great catch. This test only makes sense if sizeof(long) != sizeof(int) or else, we don't test anything and as you point out, we would overflow the long below.

>> Tools/TestWebKitAPI/Tests/WTF/MathExtras.cpp:157
>> +    unsigned long overflowUnsigned = maxUnsigned + 1;
> 
> This will overflow on certain platforms (where sizeof int == sizeof long)

Same comment here.

[Julien Chaffraix]

Created attachment 179574 [details]
Updated test for an EWS run.

[WebKit Review Bot]

Comment on attachment 179574 [details]
Updated test for an EWS run.

Clearing flags on attachment: 179574

Committed r137936: <http://trac.webkit.org/changeset/137936>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[noel gordon]

Broke mac chrome release run-api-tests tests

http://build.webkit.org/builders/Chromium%20Mac%20Release%20%28Tests%29/builds/30528

Tests that failed:
  WTF.clampToUnsigned

Note: Google Test filter = WTF.clampToUnsigned
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from WTF
[ RUN      ] WTF.clampToUnsigned
/Volumes/data/b/WebKit-BuildSlave/chromium-mac-release/build/Tools/TestWebKitAPI/TestWebKitAPI.gyp/../Tests/WTF/MathExtras.cpp:153: Failure
Value of: maxUnsigned
  Actual: 4294967295
Expected: clampTo<unsigned>(overflowUnsigned)
Which is: 0
[  FAILED  ] WTF.clampToUnsigned (0 ms)
[----------] 1 test from WTF (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (0 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] WTF.clampToUnsigned

 1 FAILED TEST

[Julien Chaffraix]

Filed bug 105253 about the failing tests (because there are several, not just one).