Bug 100915 – nrwt prints an awkward result message for missing results

• Component: New Bugs
• Status: RESOLVED FIXED

[Dirk Pranke]

nrwt prints an awkward result message for missing results

[Dirk Pranke]

Created attachment 171764 [details]
Patch

[Dirk Pranke]

let me know if anyone has any better ideas for the wording here ...

[Simon Fraser (smfr)]

Comment on attachment 171764 [details]
Patch

Do we use "baseline" synonymously with "results" elsewhere? This message could be interpreted to mean that a test can have multiple of a single type of result, not different kinds of result. If we know this is not a ref test, perhaps we could say "missing either a text or image result". Bonus points if you can say which one.

[Dirk Pranke]

(In reply to comment #3)
> (From update of attachment 171764 [details])
> Do we use "baseline" synonymously with "results" elsewhere? This message could be interpreted to mean that a test can have multiple of a single type of result, not different kinds of result. If we know this is not a ref test, perhaps we could say "missing either a text or image result". Bonus points if you can say which one.

I have no idea if there's a strong convention re: "baseline" or "expectation" or "expected result", except that I can't use the word "expected" here without it being awkward. 

We can't tell if a test is a ref test that happens to be missing the -expected.html, so it's possible we'd be missing either a text or an image result or a reference file. We do know which file types were missing, but to print out which in the summaries at the end would be awkward since we don't have different categories for missing text vs. image vs. both, etc.

Maybe "missing either a text or image result or a reference file"? Awfully wordy :(

[Ryosuke Niwa]

Comment on attachment 171764 [details]
Patch

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

> Tools/ChangeLog:10
> +        Now we will print "foo.html missing one or more baselines unexpectedly".
> +        Still a bit awkward, but better.

Do we really need to say unexpectedly? It seems redundant.

[noel gordon]

Comment on attachment 171764 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:770
> +                                MISSING: ('missing one or more baselines', 'missing one or baselines', '')}

I think you meant to write

MISSING: ('missing one or more baselines', 'missing one or more baselines', ...

[noel gordon]

(In reply to comment #4)
 
> I have no idea if there's a strong convention re: "baseline" or "expectation" or "expected result", except that I can't use the word "expected" here without it being awkward. 

Awkward due to that dangling modifier "unexpectedly" being appended to the clause.  If you can't get rid of it as rniwa@ noted, maybe try and make it parenthetical?

  "foo.html (unexpected): missing one or more test baselines."
  "foo.html (unexpected): missing one or more expected results."
  "foo.html (unexpected): missing one or more expectations."

[Dirk Pranke]

(In reply to comment #5)
> (From update of attachment 171764 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=171764&action=review
> 
> > Tools/ChangeLog:10
> > +        Now we will print "foo.html missing one or more baselines unexpectedly".
> > +        Still a bit awkward, but better.
> 
> Do we really need to say unexpectedly? It seems redundant.

The "unexpectedly" is what tells you that this was an unexpected issue in --verbose mode (as opposed to running as indicated in the TestExpectations file).

I.e., we'll print

foo.html missing one or more baselines
bar.html missing one or more baselines unexpectedly

We could flip things around, but given that there's a lot more expected failures than unexpected (and far more expected passes than either) I'm not sure that would be an improvement.

(In reply to comment #6)
> I think you meant to write
> 
> MISSING: ('missing one or more baselines', 'missing one or more baselines', ...

Whoops, you're right.

(In reply to comment #7)
> (In reply to comment #4)
> 
> > I have no idea if there's a strong convention re: "baseline" or "expectation" or "expected result", except that I can't use the word "expected" here without it being awkward. 
> 
> Awkward due to that dangling modifier "unexpectedly" being appended to the clause.  If you can't get rid of it as rniwa@ noted, maybe try and make it parenthetical?
> 
>   "foo.html (unexpected): missing one or more test baselines."
>   "foo.html (unexpected): missing one or more expected results."
>   "foo.html (unexpected): missing one or more expectations."

Good suggestion, but that doesn't really read as an improvement to me.

[noel gordon]

(In reply to comment #8) 
> > Awkward due to that dangling modifier "unexpectedly" being appended to the clause.  If you can't get rid of it as rniwa@ noted, maybe try and make it parenthetical?
> > 
> >   "foo.html (unexpected): missing one or more test baselines."
> >   "foo.html (unexpected): missing one or more expected results."
> >   "foo.html (unexpected): missing one or more expectations."
> 
> Good suggestion, but that doesn't really read as an improvement to me.

I expected that you might say that unexpectedly.

[Dirk Pranke]

Created attachment 172180 [details]
Patch

[Dirk Pranke]

Created attachment 172182 [details]
sample test run output illustrating all of the different failure messages

Okay, because I'm really compulsive about good error messages, I've totally reworked this. I've attached some sample output. Let me know if you have any feedback.

(The code is cleaner now as well, and the messages are more specific).

[Ojan Vafai]

Comment on attachment 172180 [details]
Patch

This looks much better!

[Dirk Pranke]

Committed r133380: <http://trac.webkit.org/changeset/133380>

[noel gordon]

> Okay, because I'm really compulsive about good error messages, I've totally reworked this. I've attached some sample output. Let me know if you have any feedback.

Very nice.

> (The code is cleaner now as well, and the messages are more specific).

Bonus points.  Thanks for fixing all this Dirk.  LGTM.