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

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Jędrzej Nowacki]

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.

[Jędrzej Nowacki]

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]

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]

Created attachment 55849 [details]
Fix v1.01

[WebKit Review Bot]

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]

(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 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]

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]

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 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]

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]

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 on attachment 56568 [details]
Fix v3

Clearing flags on attachment: 56568

Committed r59930: <http://trac.webkit.org/changeset/59930>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.