Adding Comments to expect() Calls

[Phoenix]

I'm fairly new to Jasmine (just started using it this morning, in
fact!). So far, I really like what I've seen, but there's one feature
that I miss from other [BT]DD frameworks, which is the ability to add
comments to assertions/expectations so that I can give myself hints as
to what went wrong when tests fail.

Unfortunately, Jasmine does not work the same way. When an expect()
fails, the only clue I have is the expected and actual values. This
can be rather frustrating, especially if an expectation fails during a
spec with multiple similar expectations.

For example (a little contrived, but work with me here):

<code>
it('can add and remove users', function( ) {
var userlist = new UserList();

username = 'testuser';
user = new User(username);

userlist.addUser(user);
expect(userlist.getUsers().length).toEqual(1);

userlist.addUser(new User('otheruser'));
userlist.removeUser(username);

expect(userlist.getUsers().length).toEqual(1);
});
</code>

Suppose after running that spec, Jasmine informed me that "Expected 2
to equal 1.". Which expectation failed? Was it the first one or the
second one? Probably the second one, but I can't be sure, and at a
minimum, I have to start by trawling through my test code just to
figure out what went wrong.

My initial suggestion would be to add an optional second parameter to
expect(), which Jasmine would output along with the failure message,
e.g.:

<code>
expect(userlist.getUsers().length, 'user object added to the
list').toEqual(1);
...
expect(userlist.getUsers().length, 'user object removed from the
list').toEqual(1);
</code>

This messes with the flow/readability of the code, though.

Some other ways of implementing this that I was thinking of:

<code>
expect(userlist.getUsers().length).toEqual(1, 'user object added to
the list');
expect(userlist.getUsers().length).toEqual(1).withFailureMessage('user
object added to the list');
</code>

What are your thoughts? Does this seem like a Jasminey way of doing
things? I'd be happy to write a patch to implement this if nobody
else is working on it.

PHX

[Corey Haines]

In general, I usually find that the need for the comment is because of
a deficiency in my example. When I take a look at the example, I
generally find that I can either split it, make it more specific, or
change the description, to get all the information I want.

For example, in your spec, I would recommend that you don't test both
#add and #remove in the same example. That would save you from this
particular case.

-Corey

> --
> You received this message because you are subscribed to the Google Groups "Jasmine" group.
> To post to this group, send email to jasmi...@googlegroups.com.
> To unsubscribe from this group, send email to jasmine-js+...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/jasmine-js?hl=en.
>
>

--
http://www.coreyhaines.com
The Internet's Premiere source of information about Corey Haines

[Phoenix]

Interesting observation. I'll keep that in mind; thanks!

[Rajan Agaskar]

You might be interested to know that we considered the following
syntax (originally proposed by Xian):

expect(userlist.getUsers().length).toEqual(1).because('when a user
object is removed, the list should equal 1');

We never implemented it, but I don't see any reason why we couldn't
going forward -- it shouldn't really affect the utility of jasmine for
any other users.

Thanks!

Rajan

[Corey Haines]

The two things that make me uncomfortable with that solution is the
reliance on specific details rather than the behavior you are looking
for. This causes potentially unnecessary checks on the state of the
object, rather than the actual behavior you are expecting.

So, having a #remove(user) method would need an example like

describe("#remove", function() {
it("removes an added user from the list", function() {
// setup
userlist.add(user);
userlist.remove(user);
expect(userlist.contains(user)).toBe(false);
// or even better
expect(userlist).toNotContain(user);

});
});

This is the expected behavior. Reaching into the list and getting the
underlying data structure and asking for its length is worrisome from
a design perspective. If you really want to have a #length property on
the userlist object, then an example like

describe("#remove", function() {
it("decrements the count by one", function() {
var originalLength = userlist.length;
userlist.remove(user);
expect(userlist.length).toEqual(originalLength - 1);
});
});

If either of these fail, you get the necessary information to find
what was wrong. So, in the case of decrementing the length, the
failure is something like:
expected: 1
got: 2
Failure: #remove - decrements the count by one

That's a pretty clear message without the need to add further description text.

I have a general rule of thumb in these cases that I use in my own
coding practices: if I find myself without the information that I need
to rapidly trace the problem, assume there is a problem in the way I'm
writing my examples, rather than in the tools I'm using. Using this, I
often find myself guided towards a better understanding of how to
write good examples. It isn't always possible to write better
examples, and sometimes the tool itself has deficiencies, but I've
found my understanding of how to test effectively, as well as my
understanding of the underlying language constructs, has benefited
greatly from questioning myself first.

Wow, that sounded a bit like a rant. Sorry, I don't have time to make
it shorter. :)
-Corey

[Gian Marco Gherardi]

+1 for that feature

[jeffContext]

Here's a valid use case for you:

$.each(requiredItems, function(i, item) {
expect(importantHash[item]).toBeDefined().because(item+' is
required');
});

because() would make for a clean way to verify that importantHash was
set with all the fields you require. Without it, you have three
inferior options: 1) write a separate example for each required
variable; 2) programmaticly add specs for the required fields; 3) have
a harder time debugging when a required field goes missing.

> > On Thu, Dec 23, 2010 at 9:05 AM, Phoenix <todofixt...@gmail.com> wrote:
> >> Interesting observation.  I'll keep that in mind; thanks!
>
> >> On Dec 22, 2010, at 4:25 PM, Corey Haines wrote:
>
> >>> In general, I usually find that the need for the comment is because of
> >>> a deficiency in my example. When I take a look at the example, I
> >>> generally find that I can either split it, make it more specific, or
> >>> change the description, to get all the information I want.
>
> >>> For example, in your spec, I would recommend that you don't test both
> >>> #add and #remove in the same example. That would save you from this
> >>> particular case.
>
> >>> -Corey
>

> >>>> For more options, visit this group athttp://groups.google.com/group/jasmine-js?hl=en.

>
> >>> --
> >>>http://www.coreyhaines.com
> >>> The Internet's Premiere source of information about Corey Haines
>
> >>> --
> >>> You received this message because you are subscribed to the Google Groups "Jasmine" group.
> >>> To post to this group, send email to jasmi...@googlegroups.com.
> >>> To unsubscribe from this group, send email to jasmine-js+...@googlegroups.com.

> >>> For more options, visit this group athttp://groups.google.com/group/jasmine-js?hl=en.

>
> >> --
> >> You received this message because you are subscribed to the Google Groups "Jasmine" group.
> >> To post to this group, send email to jasmi...@googlegroups.com.
> >> To unsubscribe from this group, send email to jasmine-js+...@googlegroups.com.

> >> For more options, visit this group athttp://groups.google.com/group/jasmine-js?hl=en.

>
> > --
> > You received this message because you are subscribed to the Google Groups "Jasmine" group.
> > To post to this group, send email to jasmi...@googlegroups.com.
> > To unsubscribe from this group, send email to jasmine-js+...@googlegroups.com.
> > For more options, visit this group athttp://groups.google.com/group/jasmine-js?hl=en.
>

> --http://www.coreyhaines.com

[Corey Haines]

In my own examples and style, I find that expectation pattern to be a
pretty strong smell that my examples are focused on the wrong thing,
and there is usually an abstraction that is waiting to come out.

If you could provide a specific example of that pattern's usage. Is
this checking the parameters to a method (lots of times an abstraction
is somewhere causing the function to require too many options, or I'm
testing incidental/unnecessary things) or a return value from
something, in which case I most likely am doing too much in the
function, or I should be testing what is done with the values.

Now, I'm not saying that you'll never, ever find a need for this. This
pattern, though, frequently points to common design/expectation
smells, especially when written in a generic fashion like that.

-Corey

> For more options, visit this group at http://groups.google.com/group/jasmine-js?hl=en.

[jeffContext]

It comes up mostly when working with model-layer objects, especially
those built up using mixins.

function PostItNote() { this.width; this.height; etc etc }
$.extend(PostItNote.prototype, UserContent, DraggableItem, NeonSign,
{
// some methods & overrides here...
});

PostItNote might contain fields tracking position, size, content, and
styles. This pattern could be used to check the right fields are being
used or, similarly, to enforce a duck-typed interface.

[Corey Haines]

Interesting. Could you give a concrete example of the expectation
code? Would you be checking that the DraggableItem fields get added to
the PostItNote?

-Corey

> For more options, visit this group at http://groups.google.com/group/jasmine-js?hl=en.

[jeffContext]

Example is getting a bit contrived, but along the lines of this:

it('should be positioned and dimensioned ', function(){
$.each(['top', 'left', 'width', 'height'], function(i, field) {
expect(myPostIt[field]).toBePixels();
});
));

it('should automatically populate all remaining styles when stuck to
blackboard', function(){
myPostIt.stick();
$.each(['fontFamily', 'fontSize', 'color', 'padding',
'backgroundColor', 'lineHeight'], function(i, field) {
expect(myPostIt[field]).toBeDefined();
});
));

On Dec 27, 5:45 pm, Corey Haines <coreyhai...@gmail.com> wrote:
> Interesting. Could you give a concrete example of the expectation
> code? Would you be checking that the DraggableItem fields get added to
> the PostItNote?
>
> -Corey
>

> ...
>
> read more »

[Corey Haines]

Ah, okay. I don't test at that level, focusing instead on the
behaviors that cause me to need those defined. If you do write
examples of that nature, then I can see your frustration.

When I get situations like that, I try to look to see whether I really
need to have those examples in that format. So, in this case, do you
really need to check that the positions are in pixels? Or, are you
really more interested in the relative position to something else,
perhaps its previous location or the mouse position?

In the case of checking #stick, does your code change the padding,
etc? Would it be enough to check that a class has been applied,
something like 'stuck' which contains those values?

-Corey

> For more options, visit this group at http://groups.google.com/group/jasmine-js?hl=en.

[Drew Chandler]

If after hearing all of Corey's great insight and advice you still
find yourself wanting this, maybe writing your own expectation could
give you some of the behavior you want.

I haven't actually run this code so it may not work at all:
this.addMatchers({
toHavePopulatedStyle: function(style) {
this.message = function() { return 'Expected ' +
jasmine.pp(this.actual) + ' to have populated the ' + style + '
style' };

return this.actual[style] !== jasmine.undefined;
}
});

> ...
>
> read more »

[jeffContext]

Corey:

> In the case of checking #stick, does your code change the padding, etc

I shouldn't have picked css names for my example, that confuses
things. I tend to follow an MVC approach when creating a javascript
UI, and I'm referring to writing model-layer stories. The model
carries data and contains business logic, it doesn't necessarily touch
the DOM. It might get instantiated in multiple places: json from an
ajax responses, written into a variable on page load, user actions.
And automatically whenever the text "Hello Kitty" is encountered
because, yes, our theoretical app is awesome. It makes sense to check
that the variously created instances contain what you're expecting
them to contain. Sometimes it's convenient to do checks on a
collection, like saying "these 12 fields should all have data by
now."

> If you do write examples of that nature, then I can see your frustration.

I'm not frustrated. :) I suppose I'm not the best advocate for this
feature because I don't care that much. But I seem to have gotten
sucked into the conversation.. may as well help hash out the idea.


Drew:

> maybe writing your own expectation could give you some of the behavior you want

toHaveProperty(propertyName) would be a pretty decent workaround for
the above example. Though I suppose it wouldn't help if you want to
apply a different matcher against a collection. eg., how you want it
to treat falsey values might differ, or you want to use a custom
matcher.


My (non-passionate) point is:
1) there are times when it is good practice to apply expectations
against a collection
2) The because() feature makes it less work to do so

Punting it because it's not high priority seems like a valid argument
against the feature--I just disagree that there's no valid use case
for it.

> ...
>
> read more »

[Trevor]

((( I apologise if this is a duplicate post, I think Google Groups ate
it... )))

I'd like to see something like this feature as well. I just started
using Jasmin today and I am really impressed. I come from a long
history of JUnit,and just started getting into Mockito / Hamcrest, and
I find that syntax extremely refreshing.

On my initial forey today I started testing and came up with some
unintuitive behavior (at least to me). I thought it was a problem with
the code I was testing because I am not yet a black belt at closures,
and it took a long time for me to track it down to my usage of Jasmin.
Let me illustrate; I started trying something like this to obtain
readable failure outupt:

describe("using describe as 'when'", function(){
var context;
describe("when context is set to 'first'", function(){
context = 'first';
it("should be 'first'", function(){
expect(context).toEqual("first");
});
});
describe("when context is set to 'last'", function(){
context = 'last';
it("should be 'last'", function(){
expect(context).toEqual("last");
});
});
});

To me this is extemely readable in the test and more or less readable
in the failure messages (I'm used to seeing the expected value before
the actual value, so that might take some time to get used to). What I
find is that (as documented) all the code for the 'describe' is run
before *any* of the code for the individual specs. What I found
unintuitive is that, in essence, *all* the code for *all* the nested
describe()s are run before any of the specs in those nested describes.
In the above code the first test fails with { Expected 'last' to equal
'first' }.

I think what I am groping for is a 'when' function that coincides with
the BDD test structuring of "given, when, then" - the body of the
describe() is the 'given', the body of the when() is the 'when', and
the body of the specs are the 'then's. I think this would improve
readability and the clear communication of intent by not cluttering up
the assertion code with "setting up the world" as the documentation
puts it. This is what I have in mind:

describe("the 'when' feature", function(){
var context;

when("the context is set to 'first'", function(){
context = "first";
}).it("should be 'first'", function(){
expect(context).toEqual("first");
});

when("the context is set to 'last'", function(){
context = 'last';
}).it("should be 'last'", function(){
expect(context).toEqual("last");
});
});


A failure would be reported something like
==========
the 'when' feature
when the context is set to 'last'
it should be 'last'
Expected 'fubar' to equal 'last'

I think this might be a good alternative to providing '.because(...)'
tacked on to the end of the assertion. That to me feels awkward in
this framework.

Inlining the specs should also be possible, like it has to be
currently in a describe(), so individual specs can be combined to test
a single set of initial conditions. In fact, it may be preferable to
inline them instead of chaining it to a 'when'; as in

describe("the 'when' feature", function(){
var obj = {
name: null
};
// avoids creating 'classes' in the global context
var ns = {};
ns.Processor = function(){
this.process = function(input){
if (input.name === undefined) {
throw new Error("bad input! no name!");
}
};
};

when("the name is set to 'bill'", function(){
obj.name = 'bill';

it("should be 'bill'", function(){
expect(obj.name).toEqual("bill");
});
it("should have one vowel and 3 consonants", function(){
expect(obj.name).toMatch(/.{4}/);
expect(obj.name).toMatch(/^[^aeiou]*[aeiou][^aeiou]*$/i);
});
it("should not contain any numbers", function(){
expect(obj.name).not.toMatch(/[0-9]/);
});
});

when("the name is undefined", function(){
obj.name = undefined;

it("should be rejected by the processor", function(){
expect(function(){new
ns.Processor().process(obj);}).toThrow();
});
});
});

I also think that the documentation should clearly state that changing
the closure context in a describe() (modifying scoped variables,
changing object state) will occur before any of the specs are run, but
that it will be ordered inside a when() as coded. If you change the
code above to use 'describe' instead of 'when', it will fail.


Anyway I want to thank the people who put a lot of hard work into this
framework, it is one of the best ones I have seen in a long time. I
especially like how I don't have to do any extra work to see *all* the
failed assertions for any one initial condition set, unlike JUnit /
Hamcrest. I am looking forward to finding out how well it fairs with
Windows Scripting (WSCRIPT).

> ...
>
> read more »

[Trevor]

[Drew Chandler]

Trevor,

Doesn't beforeEach accomplish what you want?
http://pivotal.github.com/jasmine/before-and-after.html

-Drew

> ...
>
> read more »

[Frank Schwieterman]

What Trevor is suggesting:

var context;
when("the context is set to 'first'", function(){
context = "first";
}).it("should be 'first'", function(){
expect(context).toEqual("first");
});

seems to be a shorthand for:

var context;
describe("the context is set to 'first'", function(){
beforeEach(function() {
context = "first";
})

it("should be 'first'", function(){
expect(context).toEqual("first");
});
});

I can see a shorthand like that being useful, though I don't know if
it would benefit much of my code. Normally I have a "var context;"
within the describe() block rather than before it.

[Phoenix Zerin]

I have a PoC implementation of because() working on my system.  It's definitely a little hacky; the major implementation hurdle I had to overcome is modifying the message of an ExpectationResult after it has been added to a NestedSet, which was not pretty.

I haven't tested it too extensively (does Jasmine have a test suite?), and I'm not even sure whether this would be a good fit for Jasmine, given the recent direction this thread is going.  I've attached a patch file if you'd like to try it out; it's a pretty small change.

Usage (from Rajan's suggestion):

> expect(userlist.getUsers().length).toEqual(1).because('when a user object is removed, the list should equal 1');

PHX

[Rajan Agaskar]

Phoenix: 

Thanks! Yes, Jasmine does have a test suite -- you'd want to look at our raw JS project, which doesn't include the test server, here: https://github.com/pivotal/jasmine

If we were to implement this, I imagine we'd probably do it as part of the Matcher base class -- essentially the matcher would have a because message that would override what result message it returns. Making this backwards compatible with third-party matchers might be tricky.

Thanks!

Rajan

[Tom]

I don't think it's a shorthand as you stated. It looks to me like the
same issue I ran into a few weeks ago when I was trying to test the
state of a static variable through a sequence of operations (see
http://groups.google.com/group/jasmine-js/browse_thread/thread/a8fcc2a2543eab4f#).
I think the "when" block is controlling the order of operations so the
test is not dependent on the framework for when particular things are
executed. I was able to find a (not exactly elegant) way around the
issue by doing the setting within the context of an "it" block, but it
was admittedly a little clunky. I like Trevor's idea much better. It
feels cleaner and more in tune with the spirit of the rest of
Jasmine. And if you don't need the flexibility, it seems like it
could be implemented as optional.

Tom.

> ...
>
> read more »

[Trevor]

Yes, it does; thanks for pointing that out. I was using beforeEach,
but not for this purpose. I restructured all my original tests to use
describe and beforeEach in this manner. After some thought I realised
that it would not be feasible to have the code in the 'when' block
run as I described, because we have to collect all the specs, and
therefore the code in the describe is run only once, and all up front.
I can live with this, but can we have the documentation updated to
have a more in depth scenario, and a note that the code in all the
describe blocks is run all in the same "scope" before any of the
specs?

As an example, I've been structuring my tests as

describe("a component or feature", function(){
describe("a particular facet of behavior", function(){
// declare variables and spies, etc

describe("when some condition is met", function(){
beforeEach(function(){
// set up the variables and spies to meet the
preconditions
// perform the action to simulate the condition
});

it("should have this property", function(){
expect();
expect();
});
it("should increment the success counter", function(){
expect();
});

describe("{collaborator.method()} invocation", function(){
describe("argument count", function(){
it("should be N", function(){

expect(spy.mostRecentCall.args.length).toBe(N);
});
});
describe("first argument", function(){
var arg1;
beforeEach(function(){
arg1 = spy.mostRecentCall.args[0];
});
it("should have this property", function(){
expect(arg1);
});
it("should also have this property", function(){
expect(arg1);
});
});
});
});

describe("when {collaborator.method()} throws Error",
function(){
beforeEach(function(){
// reset spy .toThrow()
// perform action, try/catch if necessary
});

describe("the error thrown in the caller's context",
function(){
it("should have this property", function(){
expect();
});
});

describe("a side effect on the object-under-test",
function() {
it("should not increment the 'success' counter",
function(){
expect();
});
});
});
});
});


Anyway I guess the short of it is when I need to describe the expected
results in more detail I find it is more useful to add another
describe/beforeEach nesting to surround a group of specs. Each spec
should check for a tightly focused aspect, whether it is a combination
of facets that make up a single state or verifies a single aspect of
behaviour. Was this the intent of how to code the tests with Jasmine?

Of course I have yet to perform any major refactoring of code tested
in this way, and I have a feeling that that might be onerous, but time
will tell I suppose.

On Jan 2, 3:37 am, Drew Chandler <andrewschand...@gmail.com> wrote:
> Trevor,
>

> Doesn't beforeEach accomplish what you want?http://pivotal.github.com/jasmine/before-and-after.html

> ...
>
> read more »

[Trevor]

Yes, the idea behind my suggestion was to provide more clarity in the
intent of the tests. Although we can use describe/beforeEach to
accomplish this, I didn't think about it previously because ... well
it just never occurred to me. As for the syntax I don't know if I my
initial proposal (chaining the spec onto the 'when') is really that
great. Although it might be a nice shortcut if we only allow a single
spec tacked on to a 'when', I think the refactoring cost would
overcome that once you need to add a couple of specs to flush out some
other conditions. After the third or fourth chained spec it looks a
little odd. Given the describe/beforeEach pattern I posted previously
I wonder if it would be sufficient to make 'when' a synonym of
'describe', perhaps with different wording in the reporting. On the
other hand maybe making 'when' a synonym of beforeEach would gain the
clarity I was seeking? Would it be feasible to add a sensible
reporting description to the when-as-beforeEach construct? I can't
seem to think of a way off hand to gain clarity in both the code and
the reporting if beforeEach could contain a description though. I
don't know if I would retire beforeEach - even if it is the same
function - because beforeEach makes more sense in a describe block
that servers to contain other describe blocks, and a 'when' (as a
synonym of beforeEach) seems more natural in a describe block that
contains specs.

And maybe a little more description in the user guide to point at
using beforeEach because all the code in all the describe blocks may
(or will) run before any contained spec - as is necessary to separate
the gathering of the specs from their execution.

On Jan 4, 11:14 pm, Tom <tku...@wingspan.com> wrote:
> I don't think it's a shorthand as you stated.  It looks to me like the
> same issue I ran into a few weeks ago when I was trying to test the

> state of a static variable through a sequence of operations (seehttp://groups.google.com/group/jasmine-js/browse_thread/thread/a8fcc2...).

> ...
>
> read more »

[Davide Mannone]

So I did it for my own for jasmine 2.0 and the related definitely-typed and I've posted it here: https://github.com/davidemannone/jasmine2.0-explained

Here is how to use it: expect(SOMETHING).toEqual(WHAT-EXPECTED).byFailReport("YOUR-CUSTOM-REPORT!");

and this reports only in case of the match fails this: "Expected SOMETHING to be WHAT-EXPECTED. >YOUR-CUSTOM-REPORT!<

For more details read the Readme.Me file.

[Davide Mannone]

So I did it for my own for jasmine 2.0 and the related definitely-typed and I've posted it here: https://github.com/davidemannone/jasmine2.0-explained

Here is how to use it: expect(SOMETHING).toEqual(WHAT-EXPECTED).byFailReport("YOUR-CUSTOM-REPORT!");

and this reports only in case of the match fails this: "Expected SOMETHING to be WHAT-EXPECTED. >YOUR-CUSTOM-REPORT!<

For more details read the Readme.Me file.