Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(58)

Issue 14794002: add more string -> unsigned number conversion unit tests (Closed)

Created:
7 years, 7 months ago by Mostyn Bramley-Moore
Modified:
7 years, 7 months ago
CC:
chromium-reviews, erikwright+watch_chromium.org, jshin+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

add more string -> unsigned number conversion unit tests Add unit tests for the following functions in base: StringToUint StringToUint64 StringToSizeT Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=201204

Patch Set 1 #

Patch Set 2 : size_t cast fixups #

Total comments: 2

Patch Set 3 : special case for 32bit size_t #

Patch Set 4 : remove redundant 64bit size_t test #

Patch Set 5 : attempt to disable windows warning, let's see if it really works #

Total comments: 2

Patch Set 6 : SIZE_MAX defines the limit of size_t #

Total comments: 2

Patch Set 7 : attempt to remove unrequired static casts #

Unified diffs Side-by-side diffs Delta from patch set Stats (+197 lines, -0 lines) Patch
M base/strings/string_number_conversions_unittest.cc View 1 2 3 4 5 6 3 chunks +197 lines, -0 lines 0 comments Download

Messages

Total messages: 23 (0 generated)
Mostyn Bramley-Moore
@Brettw: is it worth adding more unsigned number conversion tests?
7 years, 7 months ago (2013-05-01 20:01:05 UTC) #1
brettw
lgtm
7 years, 7 months ago (2013-05-02 17:51:03 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mostynb@opera.com/14794002/1
7 years, 7 months ago (2013-05-03 06:55:09 UTC) #3
Mostyn Bramley-Moore
Patchset 2 has some casting fixups, does this need to be OK'ed, or should I ...
7 years, 7 months ago (2013-05-03 07:15:54 UTC) #4
Mostyn Bramley-Moore
@brettw: ping
7 years, 7 months ago (2013-05-12 20:54:33 UTC) #5
Vitaly Buka corp
https://codereview.chromium.org/14794002/diff/16001/base/strings/string_number_conversions_unittest.cc File base/strings/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/14794002/diff/16001/base/strings/string_number_conversions_unittest.cc#newcode347 base/strings/string_number_conversions_unittest.cc:347: {"99999999999", static_cast<size_t>(99999999999), true}, it cant work on 32bit platform ...
7 years, 7 months ago (2013-05-18 08:55:17 UTC) #6
Mostyn Bramley-Moore
https://codereview.chromium.org/14794002/diff/16001/base/strings/string_number_conversions_unittest.cc File base/strings/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/14794002/diff/16001/base/strings/string_number_conversions_unittest.cc#newcode347 base/strings/string_number_conversions_unittest.cc:347: {"99999999999", static_cast<size_t>(99999999999), true}, Thanks for the review. On 2013/05/18 ...
7 years, 7 months ago (2013-05-18 10:49:06 UTC) #7
Vitaly Buka corp
lgtm
7 years, 7 months ago (2013-05-20 18:38:40 UTC) #8
Vitaly Buka corp
On 2013/05/20 18:38:40, Vitaly Buka corp wrote: > lgtm It still fails on windows. Same ...
7 years, 7 months ago (2013-05-20 18:40:52 UTC) #9
Mostyn Bramley-Moore
On 2013/05/20 18:40:52, Vitaly Buka corp wrote: > On 2013/05/20 18:38:40, Vitaly Buka corp wrote: ...
7 years, 7 months ago (2013-05-20 19:02:25 UTC) #10
Vitaly Buka corp
How did you run bots "Patch Set 1"? On 2013/05/20 19:02:25, Mostyn Bramley-Moore wrote: > ...
7 years, 7 months ago (2013-05-20 19:19:37 UTC) #11
Mostyn Bramley-Moore
On 2013/05/20 19:19:37, Vitaly Buka corp wrote: > How did you run bots "Patch Set ...
7 years, 7 months ago (2013-05-20 19:30:24 UTC) #12
Vitaly Buka corp
lgtm https://codereview.chromium.org/14794002/diff/33001/base/strings/string_number_conversions_unittest.cc File base/strings/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/14794002/diff/33001/base/strings/string_number_conversions_unittest.cc#newcode390 base/strings/string_number_conversions_unittest.cc:390: #endif Can you put this tests back to ...
7 years, 7 months ago (2013-05-20 19:42:49 UTC) #13
Vitaly Buka corp
On 2013/05/20 19:42:49, Vitaly Buka corp wrote: > lgtm please ignore this lgtm, clicked accidentally.
7 years, 7 months ago (2013-05-20 19:43:48 UTC) #14
Mostyn Bramley-Moore
https://codereview.chromium.org/14794002/diff/33001/base/strings/string_number_conversions_unittest.cc File base/strings/string_number_conversions_unittest.cc (right): https://codereview.chromium.org/14794002/diff/33001/base/strings/string_number_conversions_unittest.cc#newcode390 base/strings/string_number_conversions_unittest.cc:390: #endif On 2013/05/20 19:42:49, Vitaly Buka corp wrote: > ...
7 years, 7 months ago (2013-05-20 20:13:34 UTC) #15
Vitaly Buka (NO REVIEWS)
ok On 2013/05/20 20:13:34, Mostyn Bramley-Moore wrote: > https://codereview.chromium.org/14794002/diff/33001/base/strings/string_number_conversions_unittest.cc > File base/strings/string_number_conversions_unittest.cc (right): > > ...
7 years, 7 months ago (2013-05-20 20:40:22 UTC) #16
Vitaly Buka (NO REVIEWS)
lgtm
7 years, 7 months ago (2013-05-20 20:40:52 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mostynb@opera.com/14794002/29003
7 years, 7 months ago (2013-05-20 20:40:58 UTC) #18
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 7 months ago (2013-05-20 21:08:11 UTC) #19
Vitaly Buka (NO REVIEWS)
https://chromiumcodereview.appspot.com/14794002/diff/29003/base/strings/string_number_conversions_unittest.cc File base/strings/string_number_conversions_unittest.cc (right): https://chromiumcodereview.appspot.com/14794002/diff/29003/base/strings/string_number_conversions_unittest.cc#newcode348 base/strings/string_number_conversions_unittest.cc:348: {"2147483648", static_cast<size_t>(2147483648), true}, try 2147483648U and 99999999999U probably you ...
7 years, 7 months ago (2013-05-20 21:29:11 UTC) #20
Mostyn Bramley-Moore
https://chromiumcodereview.appspot.com/14794002/diff/29003/base/strings/string_number_conversions_unittest.cc File base/strings/string_number_conversions_unittest.cc (right): https://chromiumcodereview.appspot.com/14794002/diff/29003/base/strings/string_number_conversions_unittest.cc#newcode348 base/strings/string_number_conversions_unittest.cc:348: {"2147483648", static_cast<size_t>(2147483648), true}, On 2013/05/20 21:29:11, Vitaly Buka wrote: ...
7 years, 7 months ago (2013-05-20 21:34:57 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mostynb@opera.com/14794002/46001
7 years, 7 months ago (2013-05-20 23:37:02 UTC) #22
commit-bot: I haz the power
7 years, 7 months ago (2013-05-21 03:16:26 UTC) #23
Message was sent while issue was closed.
Change committed as 201204

Powered by Google App Engine
This is Rietveld 408576698