Bug 167537

Summary: Migrate 97 *-disabled tests to be skipped in TestExpectations
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: Tools / TestsAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, lforschler, ryanhaddad, simon.fraser
Priority: P2    
Version: Safari 10   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=20871
https://bugs.webkit.org/show_bug.cgi?id=48454
https://bugs.webkit.org/show_bug.cgi?id=58323
https://bugs.webkit.org/show_bug.cgi?id=19581
https://bugs.webkit.org/show_bug.cgi?id=10993
https://bugs.webkit.org/show_bug.cgi?id=9640
https://bugs.webkit.org/show_bug.cgi?id=7899
Attachments:
Description Flags
Patch v1
none
List of renamed tests (useful for running just these tests) none

David Kilzer (:ddkilzer)
Reported 2017-01-27 17:23:17 PST
A long time ago in a galaxy not too far away, the WebKit project used to skip layout tests by appending "-disabled" to the end of the test file. Then support for TestExpectation files was added to WebKit, giving more control over how the tests were run (or not run), and providing a way to document why they were skipped (other than by reviewing the commit log message for the rename). We should rename the remaining 99 *-disabled layout tests, and then add them into TestExpectations files if they don't currently pass. If the person moving them back into the TestExpectations file has extra time on their hands, it would be nice to group them by the reason they were disabled in the first place, and maybe reference old bugs already filed about why they were disabled. This will take a lot of time and research via git/svn log, although it could be done at a later time after moving them in bulk into LayoutTests/TestExpectations. Count: $ find LayoutTests -name \*-disabled | wc -l 99 Rename all the *-disabled files in preparation for checking in (remove the second "echo" to actually move them): $ for F in `find LayoutTests -name \*-disabled`; do G=`echo $F | sed -e 's/-disabled//'`; echo git mv $F $G; done List of files: $ find LayoutTests -name \*-disabled | tee list.txt LayoutTests/animations/font-size-using-ems.html-disabled LayoutTests/compositing/objects/composited-object-alignment.html-disabled LayoutTests/compositing/tiling/huge-layer-resize.html-disabled LayoutTests/compositing/tiling/huge-layer.html-disabled LayoutTests/css2.1/t1202-counter-10-b.html-disabled LayoutTests/css2.1/t1202-counters-10-b.html-disabled LayoutTests/css2.1/t1204-increment-00-c-o.html-disabled LayoutTests/css2.1/t1204-increment-01-c-o.html-disabled LayoutTests/css2.1/t1204-increment-02-c-o.html-disabled LayoutTests/css2.1/t1204-reset-00-c-o.html-disabled LayoutTests/css2.1/t1204-reset-01-c-o.html-disabled LayoutTests/css2.1/t1204-reset-02-c-o.html-disabled LayoutTests/dom/xhtml/level2/html/HTMLFrameElement09.xhtml-disabled LayoutTests/dom/xhtml/level3/core/documentadoptnode22.xhtml-disabled LayoutTests/dom/xhtml/level3/core/documentnormalizedocument06.xhtml-disabled LayoutTests/dom/xhtml/level3/core/documentsetdocumenturi01.xhtml-disabled LayoutTests/dom/xhtml/level3/core/documentsetdocumenturi02.xhtml-disabled LayoutTests/dom/xhtml/level3/core/domimplementationregistry12.xhtml-disabled LayoutTests/dom/xhtml/level3/core/domimplementationregistry23.xhtml-disabled LayoutTests/dom/xhtml/level3/core/nodecomparedocumentposition14.xhtml-disabled LayoutTests/dom/xhtml/level3/core/nodecomparedocumentposition15.xhtml-disabled LayoutTests/dom/xhtml/level3/core/noderemovechild03.xhtml-disabled LayoutTests/dom/xhtml/level3/core/nodereplacechild06.xhtml-disabled LayoutTests/dom/xhtml/level3/core/nodereplacechild07.xhtml-disabled LayoutTests/dom/xhtml/level3/core/nodereplacechild08.xhtml-disabled LayoutTests/editing/execCommand/create-list-1.html-disabled LayoutTests/editing/input/attributed-substring-from-range-lines.html-disabled LayoutTests/editing/pasteboard/paste-empty-startcontainer.html-disabled LayoutTests/editing/selection/inconsistent-in-removeChildNode.html-disabled LayoutTests/editing/style/5091898.html-disabled LayoutTests/fast/css/css2-system-color.html-disabled LayoutTests/fast/css/font-face-in-shadow-DOM.html-disabled LayoutTests/fast/css/limited-vendor-prefix-behavior.html-disabled LayoutTests/fast/dom/HTMLDataGridElement/DataGridColumns-basic.html-disabled LayoutTests/fast/dom/HTMLDataGridElement/DataGridColumns-dom-attributes.html-disabled LayoutTests/fast/dom/HTMLDataGridElement/DataGridColumns-dom.html-disabled LayoutTests/fast/dom/HTMLDataGridElement/DataGridDataSource-basic.html-disabled LayoutTests/fast/dom/Window/timeout-released-on-close.html-disabled LayoutTests/fast/dom/Window/window-resize-nan.html-disabled LayoutTests/fast/dom/gc-8.html-disabled LayoutTests/fast/dynamic/crash-paint-no-documentElement-renderer.html-disabled LayoutTests/fast/events/destroyed-atomic-string.html-disabled LayoutTests/fast/events/key-events-in-frame.html-disabled LayoutTests/fast/frames/iframe-scroll-page-up-down.html-disabled LayoutTests/fast/html/marquee-alternate.html-disabled LayoutTests/fast/leaks/003.html-disabled LayoutTests/fast/loader/api-test-go-to-current-back-forward-item.html-disabled LayoutTests/fast/loader/api-test-new-window-data-load-base-url.html-disabled LayoutTests/fast/loader/form-events-back-forward.html-disabled LayoutTests/fast/notifications/notifications-event-stop-propagation.html-disabled LayoutTests/fast/notifications/notifications-multi-events.html-disabled LayoutTests/fast/ruby/after-block-doesnt-crash.html-disabled LayoutTests/fast/ruby/after-table-doesnt-crash.html-disabled LayoutTests/fast/ruby/generated-after-counter-doesnt-crash.html-disabled LayoutTests/fast/ruby/generated-before-and-after-counter-doesnt-crash.html-disabled LayoutTests/fast/shadow-dom/copy-shadow-tree.html-disabled LayoutTests/fast/table/double-height-table-no-tbody.html-disabled LayoutTests/fast/text/large-text-composed-char-dos.html-disabled LayoutTests/http/tests/appcache/dynamic-entries-no-cache.html-disabled LayoutTests/http/tests/multipart/win-boundary-crash.html-disabled LayoutTests/http/tests/navigation/post-goback-repost-policy.html-disabled LayoutTests/http/tests/navigation/success200-frames-goback.html-disabled LayoutTests/http/tests/navigation/success200-frames-reload.html-disabled LayoutTests/http/tests/navigation/success200-subframeload-goback.html-disabled LayoutTests/java/lc3/ArrayMethods/object-001.html-disabled LayoutTests/java/lc3/forin/array-001.html-disabled LayoutTests/jquery/effects.html-disabled LayoutTests/js/garbage-collect-after-string-appends.html-disabled LayoutTests/js/kde/Date.html-disabled LayoutTests/js/resources/garbage-collect-after-string-appends.js-disabled LayoutTests/js/string-concatenate-outofmemory.html-disabled LayoutTests/media/video-canvas.html-disabled LayoutTests/platform/mac/plugins/pluginDocumentView-deallocated-dataSource.html-disabled LayoutTests/sputnik/Conformance/15_Native_Objects/15.1_The_Global_Object/15.1.3/15.1.3.1_decodeURI/S15.1.3.1_A2.5_T1.html-disabled LayoutTests/sputnik/Conformance/15_Native_Objects/15.1_The_Global_Object/15.1.3/15.1.3.2_decodeURIComponent/S15.1.3.2_A2.5_T1.html-disabled LayoutTests/sputnik/Conformance/15_Native_Objects/15.9_Date/15.9.3/S15.9.3.1_A5_T1.html-disabled LayoutTests/sputnik/Conformance/15_Native_Objects/15.9_Date/15.9.3/S15.9.3.1_A5_T2.html-disabled LayoutTests/sputnik/Conformance/15_Native_Objects/15.9_Date/15.9.3/S15.9.3.1_A5_T3.html-disabled LayoutTests/sputnik/Conformance/15_Native_Objects/15.9_Date/15.9.3/S15.9.3.1_A5_T4.html-disabled LayoutTests/sputnik/Conformance/15_Native_Objects/15.9_Date/15.9.3/S15.9.3.1_A5_T5.html-disabled LayoutTests/sputnik/Conformance/15_Native_Objects/15.9_Date/15.9.3/S15.9.3.1_A5_T6.html-disabled LayoutTests/sputnik/Unicode/Unicode_218/S7.6_A1.1_T5.html-disabled LayoutTests/sputnik/Unicode/Unicode_218/S7.6_A3.1.html-disabled LayoutTests/sputnik/Unicode/Unicode_218/S7.6_A3.2.html-disabled LayoutTests/sputnik/Unicode/Unicode_218/S7.6_A5.2_T5.html-disabled LayoutTests/sputnik/Unicode/Unicode_320/S7.6_A1.1_T5.html-disabled LayoutTests/sputnik/Unicode/Unicode_320/S7.6_A5.2_T5.html-disabled LayoutTests/sputnik/Unicode/Unicode_410/S7.6_A1.1_T5.html-disabled LayoutTests/sputnik/Unicode/Unicode_410/S7.6_A5.2_T5.html-disabled LayoutTests/sputnik/Unicode/Unicode_500/S7.6_A1.1_T5.html-disabled LayoutTests/sputnik/Unicode/Unicode_500/S7.6_A5.2_T5.html-disabled LayoutTests/sputnik/Unicode/Unicode_510/S7.6_A1.1_T5.html-disabled LayoutTests/sputnik/Unicode/Unicode_510/S7.6_A5.2_T5.html-disabled LayoutTests/svg/W3C-SVG-1.1/resources/filters-comptran-01-f.svg-disabled LayoutTests/svg/batik/text/textBiDi.svg-disabled LayoutTests/svg/custom/filter-source-alpha.svg-disabled LayoutTests/svg/custom/font-face-fallback.svg-disabled LayoutTests/svg/custom/js-font-test.svg-disabled LayoutTests/svg/zoom/page/zoom-svg-through-object-with-text.xhtml-disabled
Attachments
Patch v1 (115.90 KB, patch)
2017-01-28 12:39 PST, David Kilzer (:ddkilzer) 		no flags
DetailsFormatted DiffDiff
List of renamed tests (useful for running just these tests) (5.63 KB, text/plain)
2017-01-28 13:01 PST, David Kilzer (:ddkilzer) 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1 2017-01-27 17:24:47 PST
Ryan/Alexey: I'm happy to do a bulk rename of the files and add them to LayoutTests/TestExpectations, but I don't have time to categorize/sort/retest them. Is it okay just to do the rename/addition, or do you really want them categorized as they go into LayoutTests/TestExpectations?
David Kilzer (:ddkilzer)
Comment 2 2017-01-27 17:28:07 PST
Heh, there's also two resource files: LayoutTests/js/resources/garbage-collect-after-string-appends.js-disabled LayoutTests/svg/W3C-SVG-1.1/resources/filters-comptran-01-f.svg-disabled Not sure if those were renamed with corresponding test files or not.
Alexey Proskuryakov
Comment 3 2017-01-27 19:40:28 PST
I think that it's fine to just move these to Skipped list; one thing I'd do is add a comment saying that the reasons for disabling these tests should be found in Trac or svn log.
David Kilzer (:ddkilzer)
Comment 4 2017-01-28 09:49:00 PST
Output list of renamed files to a file: $ find LayoutTests -name \*-disabled | sed -e 's/-disabled//' | grep -v '/resources/' > renamed.txt
David Kilzer (:ddkilzer)
Comment 5 2017-01-28 12:39:33 PST
Created attachment 300048 [details] Patch v1
David Kilzer (:ddkilzer)
Comment 6 2017-01-28 12:41:54 PST
(In reply to comment #1) > Ryan/Alexey: I'm happy to do a bulk rename of the files and add them to > LayoutTests/TestExpectations, but I don't have time to > categorize/sort/retest them. I categorized them with bug numbers or comments on when/why they were disabled. I ran them a couple times locally with ASan enabled, but there were no interesting crashes. I think roughly half can probably be re-enabled (some marked as slow or flaky), but I don't have time to watch the bots after doing so right now.
David Kilzer (:ddkilzer)
Comment 7 2017-01-28 13:01:23 PST
Created attachment 300050 [details] List of renamed tests (useful for running just these tests) $ ./Tools/Scripts/run-webkit-tests --child-processes=4 --fully-parallel --release `cat renamed.txt`
WebKit Commit Bot
Comment 8 2017-01-28 23:32:02 PST
Comment on attachment 300048 [details] Patch v1 Clearing flags on attachment: 300048 Committed r211349: <http://trac.webkit.org/changeset/211349>
WebKit Commit Bot
Comment 9 2017-01-28 23:32:08 PST
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.