38987 – [Qt] The QScriptValue autotest should be refactored.

RESOLVED FIXED 38987

[Qt] The QScriptValue autotest should be refactored.

https://bugs.webkit.org/show_bug.cgi?id=38987

Summary [Qt] The QScriptValue autotest should be refactored.

Jędrzej Nowacki

Reported 2010-05-12 06:20:49 PDT

QScriptValue autotest have great coverage and it is really useful, but it starts to be too big. The main part of it is a generated source code (100KB). The part should be divided into several pieces to increase maintainability and compiling time (right now it doesn't profit from distributed compiling and it takes ages to compile it with MSVS2008). Summary: - tst_qscriptvalue_generated.cpp should be divided into a several files. - tests should be modified to reduce amount of optimizations taken by different compilers.

Attachments
Fix v1 (185.37 KB, patch) 2010-05-12 06:40 PDT, Jędrzej Nowacki	no flags	Details Formatted Diff Diff
Fix v1.01 (185.33 KB, patch) 2010-05-12 08:29 PDT, Jędrzej Nowacki	no flags	Details Formatted Diff Diff
Fix v2 (185.34 KB, patch) 2010-05-19 09:30 PDT, Jędrzej Nowacki	kenneth: review- kenneth: commit-queue-	Details Formatted Diff Diff
Fix v3 (185.34 KB, patch) 2010-05-20 00:19 PDT, Jędrzej Nowacki	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Jędrzej Nowacki

Comment 1 2010-05-12 06:40:34 PDT

Created attachment 55839 [details] Fix v1 Patch overview: 1. tst_qscriptvalue_generated.cpp was removed. 2. 3 new files were created with content of the removed file. - tst_qscriptvalue_generated_init.cpp - contains test data set (tested values). - tst_qscriptvalue_generated_isXXX.cpp - contains expected results and tests for QScriptValue::isXXX methods. - tst_qscriptvalue_generated_toXXX.cpp - contains expected results and tests for QScriptValue::toXXX methods. 3. typical test was changed; expected values are loaded via loops and static arrays (instead of nearby infinite numbers of insert() and operator<<() calls :-) ). 4. expected values weren't changed. 5. the same content (methods, values) is tested. 6. all tests pass.

WebKit Review Bot

Comment 2 2010-05-12 06:44:20 PDT

Attachment 55839 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 Last 3072 characters of output: [4] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:34: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:166: Missing space after , [whitespace/comma] [3] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:168: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:263: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:395: Missing space after , [whitespace/comma] [3] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:396: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:447: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:579: Missing space after , [whitespace/comma] [3] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:581: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:676: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:808: Missing space after , [whitespace/comma] [3] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:810: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:905: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:1037: Missing space after , [whitespace/comma] [3] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:1038: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:1085: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:1217: Missing space after , [whitespace/comma] [3] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:1219: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:1314: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:1446: Missing space after , [whitespace/comma] [3] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:1448: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:1543: Extra space before [ [whitespace/braces] [5] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:1675: Missing space after , [whitespace/comma] [3] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:1677: Extra space before [ [whitespace/braces] [5] Total errors found: 45 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Jędrzej Nowacki

Comment 3 2010-05-12 08:29:49 PDT

Created attachment 55849 [details] Fix v1.01

WebKit Review Bot

Comment 4 2010-05-12 08:32:27 PDT

Attachment 55849 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_isXXX.cpp:25: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_init.cpp:24: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_toXXX.cpp:24: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 3 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Jędrzej Nowacki

Comment 5 2010-05-12 08:35:32 PDT

(In reply to comment #4) > Attachment 55849 [details] did not pass style-queue: > (...) > If any of these errors are false positives, please file a bug against check-webkit-style. check-webkit-style bug 35151

Kenneth Rohde Christiansen

Comment 6 2010-05-18 10:42:32 PDT

Comment on attachment 55849 [details] Fix v1.01 \I didn't really like the "isXXX, toXXX". What about toType, isType, or maybe tst_qscriptvalue_generated_typeValidation, _typeConversion?

Jędrzej Nowacki

Comment 7 2010-05-19 09:30:43 PDT

Created attachment 56494 [details] Fix v2 (In reply to comment #6) > (From update of attachment 55849 [details]) > \I didn't really like the "isXXX, toXXX". What about toType, isType, or maybe tst_qscriptvalue_generated_typeValidation, _typeConversion? Names were changed to totype and istype.

WebKit Review Bot

Comment 8 2010-05-19 09:35:06 PDT

Attachment 56494 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_istype.cpp:25: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_totype.cpp:24: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_init.cpp:24: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 3 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Kenneth Rohde Christiansen

Comment 9 2010-05-19 10:28:01 PDT

Comment on attachment 56494 [details] Fix v2 JavaScriptCore/ChangeLog:25 + * qt/tests/qscriptvalue/tst_qscriptvalue_generated_isXXX.cpp: Added. You didn't update the ChangeLog, r-

Jędrzej Nowacki

Comment 10 2010-05-20 00:19:51 PDT

Created attachment 56568 [details] Fix v3 (In reply to comment #9) > (From update of attachment 56494 [details]) > JavaScriptCore/ChangeLog:25 > + * qt/tests/qscriptvalue/tst_qscriptvalue_generated_isXXX.cpp: Added. > You didn't update the ChangeLog, r- Oops :-)

WebKit Review Bot

Comment 11 2010-05-20 00:25:59 PDT

Attachment 56568 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_istype.cpp:25: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_totype.cpp:24: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] JavaScriptCore/qt/tests/qscriptvalue/tst_qscriptvalue_generated_init.cpp:24: Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 3 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Commit Bot

Comment 12 2010-05-21 06:42:42 PDT

Comment on attachment 56568 [details] Fix v3 Clearing flags on attachment: 56568 Committed r59930: <http://trac.webkit.org/changeset/59930>

WebKit Commit Bot

Comment 13 2010-05-21 06:42:50 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware All

OS All

Product WebKit

Component JavaScriptCore

Assignee

Jędrzej Nowacki

Reported

2010-05-12 06:20 PDT

Modified

2010-05-21 06:42 PDT History

CC List

5 users Show

URL

Keywords Qt, QtTriaged

Depends on

Blocks

36600

Dependencies

tree graph