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.
Created attachment 179548 [details] Proposed testing, feedbacks seeked on whether more testing is possible accurately.
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)
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
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.
Created attachment 179574 [details] Updated test for an EWS run.
Comment on attachment 179574 [details] Updated test for an EWS run. Clearing flags on attachment: 179574 Committed r137936: <http://trac.webkit.org/changeset/137936>
All reviewed patches have been landed. Closing bug.
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
Filed bug 105253 about the failing tests (because there are several, not just one).