Added support for arguments to the toThrow() matcher #29

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@searls

searls commented Nov 12, 2010

Because the toThrow() matcher is only currently useful if a function could be expected to throw an error because of broader-scoped state at the time the expectation is made. Because in reality, a number of exceptions are encountered because of the state of its arguments, I added support for invoking the function under test with whatever arbitrary arguments follow the expected error on the matcher.

For example, if I have a function:

var pants = function(zipper) {
  if(zipper === "open") throw new Error("No open zippers!");
};

I can now demand the above code by setting up an expectation like this:

expect(pants).toThrow(new Error("No open zippers!"),"open");

This commit doesn't (I believe) change the normal functionality of the matcher, unless there's an edge case involved with calling apply() over directly invoking the passed function that I'm not aware of.

I didn't change the message format to consider this, either, because I'm not sure whether I (or you will) like the semantic of using toThrow() this way. I'd prefer a chained semantic like this, ultimately, but I'm not sure that's currently possible in Jasmine:

expect(pants).withArguments("open").toThrow(new Error("No open zippers!"));
Added support for arguments to the built-in toThrow matcher by using …
…apply() instead of invoking the provided function directly. Backwards compatible with current toThrow; could be improved by printing the expected arguments as part of failure the messages.
@gtanner

This comment has been minimized.

Show comment Hide comment
@gtanner

gtanner Mar 16, 2011

Can be solved without any changes to the framework with:

expect(function () {
    pants("open");
}).toThrow(new Error("No open zippers!"));

My vote is that this feature is not really needed and would just pollute the API.

gtanner commented Mar 16, 2011

Can be solved without any changes to the framework with:

expect(function () {
    pants("open");
}).toThrow(new Error("No open zippers!"));

My vote is that this feature is not really needed and would just pollute the API.

@searls

This comment has been minimized.

Show comment Hide comment
@searls

searls Mar 16, 2011

Thanks for commenting it! I completely agree with you; since submitting the pull request I was pointed to the above pattern and completely forgot I'd submitted and left this pull request open.

Closing this thread since the above pattern works without a patch and is more expressive than the API addition this pull request would have included. I do think it would be helpful for the docs to have an example of this usage; now that the docs are on a github wiki I'll see about adding it.

searls commented Mar 16, 2011

Thanks for commenting it! I completely agree with you; since submitting the pull request I was pointed to the above pattern and completely forgot I'd submitted and left this pull request open.

Closing this thread since the above pattern works without a patch and is more expressive than the API addition this pull request would have included. I do think it would be helpful for the docs to have an example of this usage; now that the docs are on a github wiki I'll see about adding it.

@efi

This comment has been minimized.

Show comment Hide comment
@efi

efi Jul 12, 2011

Hi. How can I accomplish the following using the old pattern:

it ("should receive a valid JSON object", function() {
  this.answer = $.get('/list.json');
  waitsFor(function () { 
    return this.answer.responseText; // Works.
  });
  runs(function() {
    expect( this.answer.responseText ).toBeDefined(); // Works...
    expect( function () {
      JSON.parse( this.answer.responseText ); // Does not work - no access to "answer"
    });
  }).not.toThrow();
});

efi commented Jul 12, 2011

Hi. How can I accomplish the following using the old pattern:

it ("should receive a valid JSON object", function() {
  this.answer = $.get('/list.json');
  waitsFor(function () { 
    return this.answer.responseText; // Works.
  });
  runs(function() {
    expect( this.answer.responseText ).toBeDefined(); // Works...
    expect( function () {
      JSON.parse( this.answer.responseText ); // Does not work - no access to "answer"
    });
  }).not.toThrow();
});
@searls

This comment has been minimized.

Show comment Hide comment
@searls

searls Jul 12, 2011

@efi my hunch is that this is bound to something else inside the scope that expect's anonymous function actually executes. If you assign this.answer to some closure scoped variable, does it work?

it ("should receive a valid JSON object", function() {
  var answer = $.get('/list.json');
  waitsFor(function () { 
    return answer.responseText; // Works.
  });
  runs(function() {
    expect( answer.responseText ).toBeDefined(); // Works...
    expect( function () {
      JSON.parse( answer.responseText ); // Does not work - no access to "answer"
    });
  }).not.toThrow();
});

searls commented Jul 12, 2011

@efi my hunch is that this is bound to something else inside the scope that expect's anonymous function actually executes. If you assign this.answer to some closure scoped variable, does it work?

it ("should receive a valid JSON object", function() {
  var answer = $.get('/list.json');
  waitsFor(function () { 
    return answer.responseText; // Works.
  });
  runs(function() {
    expect( answer.responseText ).toBeDefined(); // Works...
    expect( function () {
      JSON.parse( answer.responseText ); // Does not work - no access to "answer"
    });
  }).not.toThrow();
});
@efi

This comment has been minimized.

Show comment Hide comment
@efi

efi Jul 13, 2011

@searls Thanks alot for your quick reply. That really did the trick. I am a bit confused but it seems I misinterpreted the documentation so that I thought that runs() can only "communicate" with the outside world via the this scope.

But all it really said was "runs() blocks share functional scope - this properties will be common to all blocks, but declared vars will not!"

efi commented Jul 13, 2011

@searls Thanks alot for your quick reply. That really did the trick. I am a bit confused but it seems I misinterpreted the documentation so that I thought that runs() can only "communicate" with the outside world via the this scope.

But all it really said was "runs() blocks share functional scope - this properties will be common to all blocks, but declared vars will not!"

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment