WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
80822
Tests for RegExp multiline failed
https://bugs.webkit.org/show_bug.cgi?id=80822
Summary
Tests for RegExp multiline failed
Hojong Han
Reported
2012-03-12 01:24:40 PDT
Testcase js1_2/regexp/RegExp_multiline.js failed Failure messages were: (multiline == true) '123\n456'.match(/^4../) = null FAILED! expected: 456 (multiline == true) 'a11\na22\na23\na24'.match(/^a../g) = a11 FAILED! expected: a11,a22,a23,a24 (multiline == true) '123\n456'.match(/.3$/) = null FAILED! expected: 23 (multiline == true) 'a11\na22\na23\na24'.match(/a..$/g) = a24 FAILED! expected: a11,a22,a23,a24 (multiline == true) 'a11\na22\na23\na24'.match(new RegExp('a..$','g')) = a24 FAILED! expected: a11,a22,a23,a24 Testcase js1_2/regexp/RegExp_multiline_as_array.js failed Failure messages were: (['$*'] == true) '123\n456'.match(/^4../) = null FAILED! expected: 456 (['$*'] == true) 'a11\na22\na23\na24'.match(/^a../g) = a11 FAILED! expected: a11,a22,a23,a24 (['$*'] == true) '123\n456'.match(/.3$/) = null FAILED! expected: 23 (['$*'] == true) 'a11\na22\na23\na24'.match(/a..$/g) = a24 FAILED! expected: a11,a22,a23,a24 (['$*'] == true) 'a11\na22\na23\na24'.match(new RegExp('a..$','g')) = a24 FAILED! expected: a11,a22,a23,a24 Testcase js1_2/regexp/beginLine.js failed Failure messages were: 123xyz'.match(new RegExp('^\d+')) = null FAILED! expected: 123 Testcase js1_2/regexp/endLine.js failed Failure messages were: xyz'.match(new RegExp('\d+$')) = null FAILED! expected: 890 These failed testcases are caused by creating RegExp instances without multiline flag, 'm'. In these testcases,"RegExp.multiline = true" is used to set multiline flag. It's not in spec as I know of. Fix testcases or JSC, otherwise just let things untouched. Which one is right?
Attachments
Patch
(7.69 KB, patch)
2012-03-12 02:07 PDT
,
Hojong Han
no flags
Details
Formatted Diff
Diff
Patch
(9.63 KB, patch)
2012-03-12 21:52 PDT
,
Hojong Han
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Gavin Barraclough
Comment 1
2012-03-12 01:47:54 PDT
RegExp.multiline is a read-only property (see ES5.1 15.10.7.4), so these are error in the test case. For these tests, probably best to change them to postfix the 'm' flag, if that will work. This reminds me, I should check that strict-mode assignment to multiline etc throws. :-)
Hojong Han
Comment 2
2012-03-12 02:07:54 PDT
Created
attachment 131298
[details]
Patch
WebKit Review Bot
Comment 3
2012-03-12 02:11:19 PDT
Attachment 131298
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/tests/mozilla/js1_2/regexp/RegExp_multiline_as_array.js:87: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/tests/mozilla/js1_2/regexp/RegExp_multiline_as_array.js:91: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/tests/mozilla/js1_2/regexp/RegExp_multiline_as_array.js:99: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/tests/mozilla/js1_2/regexp/RegExp_multiline_as_array.js:103: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/tests/mozilla/js1_2/regexp/RegExp_multiline_as_array.js:107: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/tests/mozilla/js1_2/regexp/endLine.js:64: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/tests/mozilla/js1_2/regexp/RegExp_multiline.js:87: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/tests/mozilla/js1_2/regexp/RegExp_multiline.js:91: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/tests/mozilla/js1_2/regexp/RegExp_multiline.js:99: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/tests/mozilla/js1_2/regexp/RegExp_multiline.js:103: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/tests/mozilla/js1_2/regexp/RegExp_multiline.js:107: Line contains tab character. [whitespace/tab] [5] Source/JavaScriptCore/tests/mozilla/js1_2/regexp/beginLine.js:64: Line contains tab character. [whitespace/tab] [5] Total errors found: 12 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Gavin Barraclough
Comment 4
2012-03-12 10:05:57 PDT
Comment on
attachment 131298
[details]
Patch Patch looks great, but we do require just spaces, not tab characters. Please fix the style issue, but the change looks fine.
Geoffrey Garen
Comment 5
2012-03-12 12:19:58 PDT
> In these testcases,"RegExp.multiline = true" is used to set multiline flag. It's not in spec as I know of. > Fix testcases or JSC, otherwise just let things untouched. Which one is right?
I'd suggest testing in other modern browsers and seeing what they do.
Gavin Barraclough
Comment 6
2012-03-12 12:25:36 PDT
(In reply to
comment #5
)
> > In these testcases,"RegExp.multiline = true" is used to set multiline flag. It's not in spec as I know of. > > Fix testcases or JSC, otherwise just let things untouched. Which one is right? > > I'd suggest testing in other modern browsers and seeing what they do.
Just to clarify, the comment re not being in the spec is in error; behavior here is well defined (and our implementation is correct).
Hojong Han
Comment 7
2012-03-12 21:52:55 PDT
Created
attachment 131540
[details]
Patch
Gavin Barraclough
Comment 8
2012-03-12 22:56:31 PDT
Comment on
attachment 131540
[details]
Patch Thanks for the fix - looks great!
WebKit Review Bot
Comment 9
2012-03-12 23:16:15 PDT
Comment on
attachment 131540
[details]
Patch Clearing flags on attachment: 131540 Committed
r110540
: <
http://trac.webkit.org/changeset/110540
>
WebKit Review Bot
Comment 10
2012-03-12 23:16:24 PDT
All reviewed patches have been landed. Closing bug.
Hojong Han
Comment 11
2012-03-12 23:49:46 PDT
(In reply to
comment #8
)
> (From update of
attachment 131540
[details]
) > Thanks for the fix - looks great!
Thank you for review
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug