Summary: | nrwt: move per-test result output into printing.py | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dirk Pranke <dpranke> | ||||||||
Component: | New Bugs | Assignee: | Dirk Pranke <dpranke> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, eric, ojan, tony | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Dirk Pranke
2012-07-16 19:59:48 PDT
Created attachment 152685 [details]
Patch
Created attachment 152686 [details]
add comment about removing dead code
Comment on attachment 152686 [details]
add comment about removing dead code
It looks like if you choose trace-everything (which we do on the bots, right?) we now print something for skipped tests where we didn't before. Was that intentional?
(In reply to comment #3) > (From update of attachment 152686 [details]) > It looks like if you choose trace-everything (which we do on the bots, right?) we now print something for skipped tests where we didn't before. Was that intentional? We don't run with --print trace-everything on the bots. However, even if we did, nothing would get printed for skipped tests. update_summary_with_result is only called from handle_finished_test, and so it's only called for tests that are actually run. Which means that the 'if result.type == test_expectations.SKIP:' branch is never True :). So I'll delete it. Good catch. Created attachment 152797 [details]
remove unused branch in update_summary_with_result()
Comment on attachment 152797 [details] remove unused branch in update_summary_with_result() View in context: https://bugs.webkit.org/attachment.cgi?id=152797&action=review > Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:-981 > - if result.type == test_expectations.SKIP: Maybe mention in the ChangeLog why it's safe to remove this? (In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 152686 [details] [details]) > > It looks like if you choose trace-everything (which we do on the bots, right?) we now print something for skipped tests where we didn't before. Was that intentional? > > We don't run with --print trace-everything on the bots. > > However, even if we did, nothing would get printed for skipped tests. update_summary_with_result is only called from handle_finished_test, and so it's only called for tests that are actually run. > > Which means that the 'if result.type == test_expectations.SKIP:' branch is never True :). So I'll delete it. Good catch. Ack, I'm wrong. if we skip a test because it was a reftest and --no-ref-tests was on the command line, then we will pass SKIP as a result. I will revert that part of the code, although this seems like a separate bug (we should just be skipping the reftests up front). note that I think it's okay to print this case in --print trace-everything. Committed r122897: <http://trac.webkit.org/changeset/122897> |