3

I have a very common function that I have always unit tested in the same way, but I'm wondering if there is a better solution or if it's even possible a code smell is involved. It seems like a very simple case but I have a function that clears the properties of the object. Working in JavaScript, here is a simple example:

function Dog(name, owner) {
  this.name = name;
  this.owner = owner;

  this.reset = function() {
    this.name = '';
    this.owner = '';
  };
}

var puppy = new Dog('Max', 'Timmy');
console.log(puppy.name)  // logs "Max"
puppy.reset();
console.log(puppy.name)  // logs ""

I would normally unit test by setting the properties, calling the clear function, and then asserting that the properties were indeed set back to the defaults or "cleared out". The reason I'm asking about such a simple solution is because of the dogma that unit tests should only have 1 assertion. I also think that a "reset" type function could get way out of hand when it is dealing with a large number of properties (i.e. an object that is meant to store a SPA's state).

I'm sure that I am over-thinking this but wanted to get some outside opinion/criticism for something I have been doing the same for many years. I just cannot possibly think of a better way to do this.

Another question could be, are unit tests surrounding a reset function necessary? To me they seem to almost just test the language implementation -- similar to a getter/setter property.

  • 6
    ...because of the dogma that unit tests should only have 1 assertion - that's a bit over the top in my opinion. I'd say that unit test should - preferably - only have assertions that verify one type, or one aspect of behavior (and not just everything one could think of). It ensures that one bug doesn't hide other bugs. But it doesn't mean that there must be literally one assertion per test and no more. – Konrad Morawski Dec 23 '14 at 14:48
  • While I agree with you @KonradMorawski, there still doesn't seem to be an efficient way if you are testing a function that clears a large number of properties. Maybe it's just me but I don't like having to constantly keep updating my reset() function every time I add a new property to the object. I will continue doing this, but was hoping for some enlightening/revolutionary solution :) – adam-beck Dec 23 '14 at 14:56
  • Dittos, @KonradMorawski. "one test, not one assertion" I say. I often Assert test data initial state. How else can you possibly prove reset() works if the initial state is not known/proven? FURTHER, in the sense that I'm setting "state", then there may be several objecs/object-properties to test for the proper state. – radarbob Dec 23 '14 at 15:26
  • @gnat: where do you see a function that uses other functions in the code above? – Doc Brown Dec 23 '14 at 18:18
3

Unit tests should cover logic, and as a matter of fact, reset doesn't contain any logic - there is no ifs, no switches, no loops in it - basically, no conditional statements of any type.

And yes, it means that testing it sort of boils down to testing JavaScript as such, as you say. Set a, b, and c to empty strings! Have a, b and c been set to empty strings? Good. Good JavaScript!

So, given there's no logic, why would we want unit test coverage here at all?

I guess we'd wish to have it in order to protect ourselves against the scenario in which you're adding another property to the class, but then forget to reset it in your reset function.

The problem here is that you would also have to update your unit test to reveal this bug, and if you forgot about updating your reset function, it stands to reason you would have failed to update testReset, too.

Or your little special function that returns all the contents of your singleton, nicely packed for testing purposes.

One possible alternative would be to use reflection (in case of JavaScript, it's just iterating over properties of course) for resetting all properties in existence, and then only unit test it as a universal utility, even on an arbitrary stub class.

Of course you're likely to get into more problems if you want to actually preserve the value of some of your properties rather than wipe everything clean.

All in all, it's a difficult task because that's a singleton you have to reset. Singletons are notoriously bad for testability.

Misko Hevery devoted a series of articles and presentations to that. See:

  • 1
    Singleton? Did I miss something in the code above? – Doc Brown Dec 23 '14 at 18:22
  • 1
    -1 Because it has logic - just because it does not have any ifs does not mean it doesn't do anything. And at the very least a simple unit test will protect you from accidentally modifying any established functionality. – Maurycy Dec 23 '14 at 20:48
  • @DocBrown ctrl + F and "singleton" would quickly reveal where I got this crazy singleton idea from :) Not in the code above, but the OP mentions this contraint in comments under Cramps's answer: "My example may have been a little too simplistic, but the object is a singleton (Angular service) that is injected into multiple controllers" – Konrad Morawski Dec 24 '14 at 8:56
  • @MaurycyZarzycki - "at the very least a simple unit test will protect you from accidentally modifying any established functionality" - did I not acknowledge that in my answer? "I guess we'd wish to have it [the test] in order to protect ourselves against the scenario in which you're adding another property to the class, but then forget to reset it in your reset function." I don't mind a downvote, there's no pleasing everyone, but it seems that at best you've only skim read my answer. – Konrad Morawski Dec 24 '14 at 8:58
  • 1
    @DocBrown but what follows immediately after this rhetorical question is: I guess we'd wish to have it in order to (...). And thanks. Well, that's the problem about this type of tests: since they have to mirror the implementation in order to work in the first place, they're not that much of a safety net – Konrad Morawski Dec 24 '14 at 9:06
5

You are over-thinking the "one assertion" rule/guideline.

The reason for the rule is that a test case should test only one "behaviour" of a class. By testing only one behaviour per test case, you make it much easier on yourself to pinpoint where an error got introduced when a test case starts failing.

The problem is that it is hard to tell when that basic rule of testing only one behaviour is being violated, also because people might disagree what falls under "one behaviour".
However, in the large majority of cases, the one behaviour that you want to test can be verified with a single assert statement. For that reason, the rule is being stated as "you shall have only a single assert".
For the minority of cases where a single, indivisible action of the class has multiple outputs and/or side effects (like your reset method), it is perfectly fine to have multiple assert statements. In fact, you are still asserting just one thing, namely that all properties have been cleared.

2

An alternative approach is two tests, one for each component of your reset function. Or more, if you have more. These would be pretty simple to create.

A key point of tests is to know what is failing when something fails. If you test the "reset" method and it fails you want to know what failed. While not super important in your example, other situations like this might be much less clear which part is failing.

By testing each individual variable, as long as your workflow is writing a failing test when you add new variables, say Breed to your dog, you will be forced to add Breed to the reset function because you immediately have a failing test.

0

Probably instead of deciding based on some authorities view of best practices, it's better to remember why you are writing unit tests - such as you want to save time later by catching bugs early in development, prevent bugs from reaching users, and document the code for developers (and possibly for others).

The deciding principle, then, is whether the time spent writing the tests is justified in achieving those goals. Of course, this is still a guess based on limited information, but that's the best that can be done.

How long will it take you to figure out the actual bug if you write one test versus N tests? Which way will be more stable in the face of changing requirements? In this case I personally would estimate that writing a large number of tests probably won't be worth the time, plus the semantics of "clearing out the properties" apparently implies that it should clear out future properties as well. Note that this could technically be written in a single assertion anyway, for example:

assert.isTrue(myLibrary.propertyCollection(objectUnderTest).All(function(x) {x==='';}));

Whether this makes sense in your situation is something you can answer better than I can.

-1

You could add a function that returns the properties in their default state enclosed in an object and then another that returns their current state. Compare the returning values of each of those functions and assert accordingly.

In this particular example, you should be able to assert true that puppy.reset().getNameAndOwner() should be equal to puppy.getDefaultState(). (There's a link for Object comparison at the bottom of my post. Do NOT use == or ===)

See below

function Dog(name, owner) {
   this.name = name || '';
   this.owner = owner || '';

   this.reset = function() {
       this.name = '';
       this.owner = '';
   };

   this.getNameAndOwner = function() {
       var name = this.name
           owner = this.owner;
       return { name: name, owner: owner };
   }

   this.defaultState = function() {
       var emptyName = '',
           emptyOwner = '';
       return { name: emptyName, owner: emptyOwner };
   }
}

How to compare objects:

Object comparison in JavaScript
How to determine equality for two JavaScript objects?

  • My example may have been a little too simplistic, but the object is a singleton (Angular service) that is injected into multiple controllers. Therefore, there is no constructor function. – adam-beck Dec 23 '14 at 14:33
  • @AdamBInfinity Well, in that case you could add a function that returns the state of each of the properties you're interested in, all of them enclosed in an object. In this case it would result in something like { name: 'Max', owner: 'Timmy' }. Then you can compare puppy.getNameAndOwner() and compare it to new Dog().getNameAndOwner() Or, if you don't have access to the constructor, add another function that returns the default state of those properties enclosed in an object, in this case { name: '', owner: '' }, and compare that to puppy.reset().getNameAndOwner() – Cramps Dec 23 '14 at 14:44
  • 2
    This would work but it seems like more effort with no added benefits to accomplish the same thing as what I'm already doing. And in my case, I have ~8 state properties being maintained. I wouldn't want a function getProp1AndProp2AndProp3. Plus you are introducing functionality strictly for testing. – adam-beck Dec 23 '14 at 14:52
  • That would make the reset-test require a specific implementation of the constructor function, which is a dependency you would want to avoid. Changing the constructor suddenly breaks a completely irrelevant test. – Arve Systad Dec 23 '14 at 14:53
  • 1
    @ArveSystad You're right. I changed it now since it wasn't a very practical approach. – Cramps Dec 23 '14 at 14:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.