As a result of discussion in bug https://bugs.webkit.org/show_bug.cgi?id=113742 we're moving spelling tests from layout tests to unit tests.
Created attachment 205583 [details] Proposed patch
Comment on attachment 205583 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=205583&action=review > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:54 > +static size_t contextMenuItemsNumber = 0; Please use unsigned as this is the type returned by eina_list_count(). > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:329 > + size_t numberItemsWithoutSpellCheck = contextMenuItemsNumber; unsigned > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:338 > + size_t numberItemsWithSpellCheck = contextMenuItemsNumber; unsigned > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:342 > + wasContextMenuShown = false; I don't think we should reset the boolean at the end of the test. The next test should set the boolean value before using it instead. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:371 > + wasContextMenuShown = false; Ditto.
Comment on attachment 205583 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=205583&action=review > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:328 > + waitUntilTrue(wasContextMenuShown); You need to ASSERT_TRUE iirc. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:337 > + waitUntilTrue(wasContextMenuShown); You need to ASSERT_TRUE iirc. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:356 > + waitUntilTrue(wasContextMenuShown); You need to ASSERT_TRUE iirc. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:357 > + size_t numberItemsWithoutSpellCheck = contextMenuItemsNumber; unsigned. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:361 > + // Testing how many items are in context menu when multiword are selected. "multiple words" > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:366 > + waitUntilTrue(wasContextMenuShown); You need to ASSERT_TRUE iirc. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:367 > + size_t numberItemsWithSpellCheck = contextMenuItemsNumber; unsigned > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:385 > + waitUntilTrue(wasContextMenuShown); You need to ASSERT_TRUE iirc. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:386 > + size_t numberItemsWithoutSpellCheck = contextMenuItemsNumber; unsigned > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:395 > + waitUntilTrue(wasContextMenuShown); You need to ASSERT_TRUE iirc. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:396 > + size_t numberItemsWithSpellCheck = contextMenuItemsNumber; unsigned > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:400 > + wasContextMenuShown = false; boolean reset > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:414 > + waitUntilTrue(wasContextMenuShown); You need to ASSERT_TRUE iirc. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:415 > + size_t numberItemsWithoutSpellCheck = contextMenuItemsNumber; unsigned
Created attachment 205592 [details] Proposed patch version 2
Attachment 205592 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/efl/tests/resources/spelling_selection_tests.html', u'Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp']" exit_code: 1 Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:54: Omit int when using unsigned [runtime/unsigned] [1] Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:331: Omit int when using unsigned [runtime/unsigned] [1] Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:340: Omit int when using unsigned [runtime/unsigned] [1] Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:350: Tab found; better to use spaces [whitespace/tab] [1] Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:359: Omit int when using unsigned [runtime/unsigned] [1] Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:369: Omit int when using unsigned [runtime/unsigned] [1] Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:379: Tab found; better to use spaces [whitespace/tab] [1] Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:388: Omit int when using unsigned [runtime/unsigned] [1] Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:398: Omit int when using unsigned [runtime/unsigned] [1] Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:408: Tab found; better to use spaces [whitespace/tab] [1] Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:417: Omit int when using unsigned [runtime/unsigned] [1] Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:428: Omit int when using unsigned [runtime/unsigned] [1] Total errors found: 12 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 205594 [details] Proposed patch version 3
Comment on attachment 205594 [details] Proposed patch version 3 View in context: https://bugs.webkit.org/attachment.cgi?id=205594&action=review Looks fine. r=me with nits. Please fix the nits and let Grzegorz take a look before landing. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:340 > + unsigned numberItemsWithSpellCheck = contextMenuItemsNumber; nit: Local variable not needed. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:369 > + unsigned numberItemsWithSpellCheck = contextMenuItemsNumber; nit: Local variable not needed. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:398 > + unsigned numberItemsWithSpellCheck = contextMenuItemsNumber; nit: Local variable not needed. > Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:428 > + unsigned numberItemsWithSpellCheck = contextMenuItemsNumber; nit: Local variable not needed.
Comment on attachment 205594 [details] Proposed patch version 3 View in context: https://bugs.webkit.org/attachment.cgi?id=205594&action=review > Source/WebKit2/ChangeLog:3 > + Porting spellcheck tests from layout tests to unit tests. Also, would you change the title? It sounds like you're removing layout tests and adding API tests instead. Just explain what kind of API test you're adding and leave layout tests out of this.
Created attachment 205603 [details] Proposed patch version 4
Comment on attachment 205603 [details] Proposed patch version 4 Attachment 205603 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/854431 New failing tests: fast/js/global-constructors-attributes-shared-worker.html fast/js/global-constructors-attributes-dedicated-worker.html
Created attachment 205623 [details] Archive of layout-test-results from APPLE-EWS-1 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: APPLE-EWS-1 Port: win-future Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit
Comment on attachment 205603 [details] Proposed patch version 4 Nothing comes to mind after Chris' review :) Thanks guys for supporting those tests!
Comment on attachment 205603 [details] Proposed patch version 4 Clearing flags on attachment: 205603 Committed r152153: <http://trac.webkit.org/changeset/152153>