Handle should.throw() assertion with no argument properly

Handle should.throw() assertion with no argument properly

Fix a bug spitting '${expected}' in the error message when the no
argument is given. Also print out the message associated with the
actual error.

BUG=676377
TEST=LayoutTests/webaudio/unit-tests/audit.html

Committed: https://crrev.com/0cafa0b70e49ee5e694be08f19785c9b79e8e49a
Cr-Commit-Position: refs/heads/master@{#440468}

[hongchan, 2016-12-22 17:38:50]

hongchan@chromium.org changed reviewers:
+ rtoy@chromium.org

[hongchan, 2016-12-22 17:38:50]

PTAL.

[Raymond Toy, 2016-12-22 17:55:00]

lgtm with nits

https://codereview.chromium.org/2595393003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/webaudio/resources/audit.js (right):

https://codereview.chromium.org/2595393003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/resources/audit.js:273: if
(this._expected === null) {
What if someone did 

should(...).throw(undefined)

And is that what you want?

https://codereview.chromium.org/2595393003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/resources/audit.js:277: + error.message
+ '".';
Nit: Can we actually throw anything other than an exception?  If not, then maybe
we can just remove "an exception of"?

https://codereview.chromium.org/2595393003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/resources/audit.js:282: + error.message
+ '".';
Nit:  Similarly, can we just say "threw expected ${expected}", just to be
slightly more concise?

[hongchan, 2016-12-22 18:10:31]

https://codereview.chromium.org/2595393003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/webaudio/resources/audit.js (right):

https://codereview.chromium.org/2595393003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/resources/audit.js:273: if
(this._expected === null) {
On 2016/12/22 17:55:00, Raymond Toy wrote:
> What if someone did 
> 
> should(...).throw(undefined)
> 
> And is that what you want?

Giving |undefined| as an argument will do nothing. The expected value will still
be |null|. The |null| value of the expected means that user did not provide
anything to expect. We can certainly introduce enums to clarify this, but I
don't think it's necessary.

https://codereview.chromium.org/2595393003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/resources/audit.js:277: + error.message
+ '".';
On 2016/12/22 17:55:00, Raymond Toy wrote:
> Nit: Can we actually throw anything other than an exception?  If not, then
maybe
> we can just remove "an exception of"?

Good call. Done.

https://codereview.chromium.org/2595393003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/resources/audit.js:282: + error.message
+ '".';
On 2016/12/22 17:55:00, Raymond Toy wrote:
> Nit:  Similarly, can we just say "threw expected ${expected}", just to be
> slightly more concise?
> 

Sure. Done.

[hongchan, 2016-12-22 18:10:37]

The CQ bit was checked by hongchan@chromium.org

[hongchan, 2016-12-22 18:10:38]

The patchset sent to the CQ was uploaded after l-g-t-m from rtoy@chromium.org
Link to the patchset: https://codereview.chromium.org/2595393003/#ps20001
(title: "Addressing feedback")

[commit-bot: I haz the power, 2016-12-22 18:10:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-22 19:24:24]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1482430237610250,
"parent_rev": "921798e3ddbbb82c152a525c6d643293bcd85810", "commit_rev":
"18b6b7160070a8ce7e245fe45380b99ad0c81af0"}

[commit-bot: I haz the power, 2016-12-22 19:24:48]

Description was changed from

==========
Handle should.throw() assertion with no argument properly

Fix a bug spitting '${expected}' in the error message when the no
argument is given. Also print out the message associated with the
actual error.

BUG=676377
TEST=LayoutTests/webaudio/unit-tests/audit.html
==========

to

==========
Handle should.throw() assertion with no argument properly

Fix a bug spitting '${expected}' in the error message when the no
argument is given. Also print out the message associated with the
actual error.

BUG=676377
TEST=LayoutTests/webaudio/unit-tests/audit.html

Review-Url: https://codereview.chromium.org/2595393003
==========

[commit-bot: I haz the power, 2016-12-22 19:24:49]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-22 19:30:14]

Description was changed from

==========
Handle should.throw() assertion with no argument properly

Fix a bug spitting '${expected}' in the error message when the no
argument is given. Also print out the message associated with the
actual error.

BUG=676377
TEST=LayoutTests/webaudio/unit-tests/audit.html

Review-Url: https://codereview.chromium.org/2595393003
==========

to

==========
Handle should.throw() assertion with no argument properly

Fix a bug spitting '${expected}' in the error message when the no
argument is given. Also print out the message associated with the
actual error.

BUG=676377
TEST=LayoutTests/webaudio/unit-tests/audit.html

Committed: https://crrev.com/0cafa0b70e49ee5e694be08f19785c9b79e8e49a
Cr-Commit-Position: refs/heads/master@{#440468}
==========

[commit-bot: I haz the power, 2016-12-22 19:30:15]

Patchset 2 (id:??) landed as
https://crrev.com/0cafa0b70e49ee5e694be08f19785c9b79e8e49a
Cr-Commit-Position: refs/heads/master@{#440468}