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

Issue 55983002: Make gtest always use simple internal regex engine. (Closed)

Created:
7 years, 1 month ago by mithro-old
Modified:
7 years ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Make gtest always use simple internal regex engine. In order to allow regex matches in gtest to be shared between Windows and other systems, we tell gtest to always use it's internal engine. The syntax supported by the internal engine initially looks like a subset of POSIX regexs. However character class shortcuts are not valid POSIX. Even more confusingly the system POSIX regex function often defines extra features not actually part of the standard allowing regex that work on Linux fail on Mac OS X. A search through the code base did not reveal any locations where features not supported by the internal regex engine where used. See bug https://code.google.com/p/chromium/issues/detail?id=317224 for more detailed description. BUG=317224 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=241500

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : Fixing tests which had no longer supported regexes. #

Patch Set 4 : Fixing more tests which rely on '\n' being included in .* #

Patch Set 5 : Updating memcheck_analyze.py #

Patch Set 6 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -11 lines) Patch
M base/message_loop/message_pump_io_ios_unittest.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M base/message_loop/message_pump_libevent_unittest.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M base/process/memory_unittest.cc View 1 2 2 chunks +3 lines, -5 lines 0 comments Download
M base/tools_sanity_unittest.cc View 1 2 3 4 5 1 chunk +5 lines, -4 lines 0 comments Download
M testing/gtest.gyp View 1 2 1 chunk +10 lines, -0 lines 0 comments Download

Messages

Total messages: 24 (0 generated)
mithro-old
The following targets all depend on the gtest.gyp target and hence need to be checked. ...
7 years, 1 month ago (2013-11-01 17:42:52 UTC) #1
mithro-old
Hi people, I've been trying to create more unit tests for blink as part of ...
7 years, 1 month ago (2013-11-01 18:29:50 UTC) #2
mithro-old
On 2013/11/01 18:29:50, mithro wrote: > Hi people, > > I've been trying to create ...
7 years, 1 month ago (2013-11-09 11:21:35 UTC) #3
awong
LGTM Sorry for the delay. I do like this change (and your change description). Before ...
7 years, 1 month ago (2013-11-09 17:14:22 UTC) #4
awong
LGTM Sorry for the delay. I do like this change (and your change description). Before ...
7 years, 1 month ago (2013-11-09 17:14:24 UTC) #5
mithro-old
On 2013/11/09 17:14:24, awong wrote: > LGTM > > Sorry for the delay. I do ...
7 years, 1 month ago (2013-11-10 05:06:58 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mithro@mithis.com/55983002/190001
7 years ago (2013-12-04 02:25:34 UTC) #7
commit-bot: I haz the power
Retried try job too often on android_aosp for step(s) compile http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_aosp&number=31591
7 years ago (2013-12-04 03:12:04 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mithro@mithis.com/55983002/190001
7 years ago (2013-12-04 04:16:38 UTC) #9
commit-bot: I haz the power
Retried try job too often on android_aosp for step(s) compile http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_aosp&number=31613
7 years ago (2013-12-04 04:51:36 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mithro@mithis.com/55983002/210001
7 years ago (2013-12-05 02:17:55 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mithro@mithis.com/55983002/210001
7 years ago (2013-12-05 02:40:53 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mithro@mithis.com/55983002/230001
7 years ago (2013-12-05 03:47:40 UTC) #13
commit-bot: I haz the power
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=39571
7 years ago (2013-12-05 03:59:48 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mithro@mithis.com/55983002/230001
7 years ago (2013-12-05 04:44:08 UTC) #15
commit-bot: I haz the power
Change committed as 238913
7 years ago (2013-12-05 07:31:51 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mithro@mithis.com/55983002/260001
7 years ago (2013-12-08 23:48:01 UTC) #17
commit-bot: I haz the power
Change committed as 239423
7 years ago (2013-12-09 01:38:22 UTC) #18
sadrul
Hi. This was reverted due to valgrind failure in linux, cros and mac bots. http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28valgrind%29%282%29/builds/33131 ...
7 years ago (2013-12-10 05:45:10 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/mithro@mithis.com/55983002/320001
7 years ago (2013-12-18 02:35:15 UTC) #20
commit-bot: I haz the power
Change committed as 241500
7 years ago (2013-12-18 05:31:36 UTC) #21
mithro-old
Hi Mark, Can you please take another look.
7 years ago (2013-12-19 10:15:45 UTC) #22
mithro-old
Hi Mark, Can you please take another look.
7 years ago (2013-12-19 10:15:45 UTC) #23
mithro-old
7 years ago (2013-12-19 10:16:15 UTC) #24
Message was sent while issue was closed.
On 2013/12/19 10:15:45, mithro wrote:
> Hi Mark,
> 
> Can you please take another look.

Please ignore that message, was wrong CL.

Powered by Google App Engine
This is Rietveld 408576698