Bug 189088

Summary: Make it possible to mark tests as leaks in TestExpectations
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: New BugsAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, ews-watchlist, glenn, jbedard, jonlee, simon.fraser, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 186214    
Attachments:
Description Flags
Patch
none
Patch
none
Patch jonlee: review+

Simon Fraser (smfr)
Reported 2018-08-29 10:25:48 PDT
Make it possible to mark tests as leaks in TestExpectations
Attachments
Patch (17.81 KB, patch)
2018-08-29 10:28 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Patch (17.82 KB, patch)
2018-08-29 13:02 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Patch (21.39 KB, patch)
2018-08-29 15:11 PDT, Simon Fraser (smfr) 		jonlee: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2018-08-29 10:28:41 PDT
Created attachment 348411 [details] Patch
Simon Fraser (smfr)
Comment 2 2018-08-29 10:29:34 PDT
Needs the patch from bug 189067.
Jonathan Bedard
Comment 3 2018-08-29 12:42:51 PDT
Is the intention to have this enabled by default or to have a queue explicitly testing it?
Simon Fraser (smfr)
Comment 4 2018-08-29 12:59:36 PDT
Enabled by default on some platforms.
Simon Fraser (smfr)
Comment 5 2018-08-29 13:02:29 PDT
Created attachment 348424 [details] Patch
Alexey Proskuryakov
Comment 6 2018-08-29 13:55:53 PDT
Comment on attachment 348424 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348424&action=review > Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:75 > +Bug(test) failures/expected/leaky-reftest.html [ ImageOnlyFailure Leak ] I still think that "Leak" is a confusing name for this - I definitely wouldn't think that it has any relationship to document leaks.
Simon Fraser (smfr)
Comment 7 2018-08-29 14:51:50 PDT
I went with "leak" because "abandoned document" is too long, we may want to track other types of leaked objects (e.g. Frames) later, and it's consistent with the "world leak" term that WebKit has used historically.
Simon Fraser (smfr)
Comment 8 2018-08-29 15:11:57 PDT
Created attachment 348435 [details] Patch
Simon Fraser (smfr)
Comment 9 2018-08-30 09:32:14 PDT
Review ping?
Jon Lee
Comment 10 2018-08-30 09:53:41 PDT
Comment on attachment 348435 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348435&action=review > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:-248 > - 'Leak': 'LEAK', There are enough here that maybe the list should just be alphabetized...? > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:868 > expected_results.add(PASS) Not part of your patch, but it seems like this is wrong. > Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:136 > self.assertEqual(TestExpectations.remove_pixel_failures(set([PASS, IMAGE, CRASH])), set([PASS, CRASH])) Maybe add some test cases with LEAK here?
Simon Fraser (smfr)
Comment 11 2018-08-31 13:03:32 PDT
https://trac.webkit.org/r235564
Radar WebKit Bug Importer
Comment 12 2018-08-31 13:04:18 PDT
<rdar://problem/43952197>
Note You need to log in before you can comment on or make changes to this bug.