Bug 189293 – Log when leak detection changes the test result

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Fraser (smfr)]

Log when leak detection changes the test result

[Simon Fraser (smfr)]

Created attachment 348883 [details]
Patch

[Jonathan Bedard]

Comment on attachment 348883 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=348883&action=review

> Tools/ChangeLog:10
> +        When you have an "Leak" expectation in TestExpectations, it's confusing to see a test unexpectedly pass,
> +        but then show up as an expected fail at the end (because leak detection happens at the end of a shard).
> +        So log when leak detection changes a test result.

Does this change actually address the fact that a test will show up as an unexpected pass until the leak detection happens? Or does it just log when the result changes after leak detection occurs?

It looks like this does the second. I'd like to eventually do the first, but this seems like a decent mitigation.

[Simon Fraser (smfr)]

(In reply to Jonathan Bedard from comment #2)
> Comment on attachment 348883 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=348883&action=review
> 
> > Tools/ChangeLog:10
> > +        When you have an "Leak" expectation in TestExpectations, it's confusing to see a test unexpectedly pass,
> > +        but then show up as an expected fail at the end (because leak detection happens at the end of a shard).
> > +        So log when leak detection changes a test result.
> 
> Does this change actually address the fact that a test will show up as an
> unexpected pass until the leak detection happens? Or does it just log when
> the result changes after leak detection occurs?
>
> It looks like this does the second. I'd like to eventually do the first, but
> this seems like a decent mitigation.

The latter. Leak detect is async, so the only other option would be to log something like: "fast/css/user-drag-none.html passed, but might leak", and then something when the leak is detected.

[Jonathan Bedard]

(In reply to Simon Fraser (smfr) from comment #3)
> (In reply to Jonathan Bedard from comment #2)
> > Comment on attachment 348883 [details]
> > ...
> 
> The latter. Leak detect is async, so the only other option would be to log
> something like: "fast/css/user-drag-none.html passed, but might leak", and
> then something when the leak is detected.

So I like the other option for tests that are expected to leak.

There is a third (but much more painful) option: we could defer printing ALL results until leak checking is finished. But that's a pretty non-trivial and controversial change.

[Simon Fraser (smfr)]

Created attachment 349084 [details]
Patch

[Simon Fraser (smfr)]

Output: 

Unexpected leak:
[1/1] fast/css/user-drag-none.html passed
  fast/css/user-drag-none.html -> changed by leak detection from a pass (expected) to a leak (unexpected)

Expected leak:
[1/1] fast/css/user-drag-none.html passed (leak detection is pending)
  fast/css/user-drag-none.html -> changed by leak detection from a pass (unexpected) to a leak (expected)

[WebKit Commit Bot]

Comment on attachment 349084 [details]
Patch

Rejecting attachment 349084 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 349084, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as [e-mail]...
Fetching: https://bugs.webkit.org/attachment.cgi?id=349084&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=189293&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Processing patch 349084 from bug 189293.
Fetching: https://bugs.webkit.org/attachment.cgi?id=349084
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Jon Lee']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 4 diffs from patch file(s).
patching file Tools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py
Hunk #1 FAILED at 200.
1 out of 1 hunk FAILED -- saving rejects to file Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py.rej
patching file Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
Hunk #1 succeeded at 815 (offset 14 lines).
patching file Tools/Scripts/webkitpy/layout_tests/views/printing.py

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Jon Lee']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/9120256

[Simon Fraser (smfr)]

https://trac.webkit.org/r235771

[Radar WebKit Bug Importer]

<rdar://problem/44204528>