Story 1
I still remember the month long final project from my first ever CS class. Actually, I remember almost nothing from the project except for a bug that caused me to have to throw away my first three weeks of work. I had just learned about enumerations in Pascal and thought they were the neatest thing since sliced bread. I designed my entire project around enumerations. Users would enter their menu choices via enumerations. I would display the enumerations as options. All of this depended very heavily on my understanding of enumerations which, as it turned out, was in no way related to what enumerations actually are.
Unfortunately, I didn't find this out until I had spent two weeks writing the entire program and tried running it. It didn't work at all. Like any good developer I randomly changed various lines of code until it worked. A week later, with the project deadline looming, my project was not any closer to working. It was time for drastic action. Since I couldn't seem to get the enumerations to do what they were "supposed" to do, I actually read up on them. Imagine my dismay when I realized that all the assumptions that I had made were wrong. And not just a little bit wrong. The reality (enums are a typesafe way to have named constants) and my vision (they'll do everything I want, including cooking dinner and solving the halting problem) were completely unrelated. I spent the rest of the day griping and complaining about why would anybody create such a useless design feature. When I realized that that wasn't getting me any closer to a program that I could actually turn in, I finally sat down and rewrote the entire program using only Pascal features that I actually understood, and eschewed enumerations entirely.
Story 2
The other day a friend asked me what I knew about Java Serialization. After asking for details, it turns out that he was working on a project where they were doing bit-diddling on the Serialized output of an object. One of the assumptions that they had was that if you had an objects instance1 and instance2, and you serialize both of them, that the serialized byte data would be identical if instance1 and instance2's fields are all identical. They had run a bunch of experiments and it seemed like their assumption wasn't always holding. I think the question to me was hoping that I would point out something they had missed that would give them an easy fix.
Upon check the Serialization Spec, it turns out their assumption was not valid. My suggested solution was to not use Java's Serialization, but rather write their own conversion to bytes that would fit their specific needs. This may seem like reinventing the wheel, but as I've mentioned before, I believe that rolling your own is often the way to go.
Story 3
On a project I once worked on, the decision was made that all data was going to be handled as XML. All of the data was stored as an XML document in a single column in a database. To process individual data items, XPath queries would be made. HTML reports would be created by running giant XSLT transforms on the data. The assumption was that because XML has such great support, it would be easy to add new data (just turn it into XML and merge it with the other data), and create new reports (just write a new XSLT).
The reality is that just because you can create a general tool to handle syntactically correct XML, you still need to customize the logic for the semantics. Which meant every new data source added still required code and logic changes. As for generating the HTML via XSLT transformations, its very difficult to use modern HTML features, like Ajax, this way. And by making such a large portion of the code base XSLT and XPath, we couldn't take advantage of the IDE's ability to do easy code completion, navigation, and refactoring, or leverage the development team's OO skill to create small, easily testable, and reusable components. And then there's the performance issues, both in time and space, that were encountered with trying to manipulate and store large quantities of large XML documents.
Moral
The moral of these stories is that you need to know your tools. Enumerations, Serialization, and XML are all great, when used appropriately. When not... well, invalid assumptions lead to bugs. Invalid assumptions about crucial components of your software lead to large scale rewrites. To avoid the large scale rewrite, we often have a period of denial where we try to make the unworkable work. The worst case is that with enough bubble gum, chicken wire, and ingenuity we do make it work, at least some of the time. This results in a maintenance nightmare keeping it working and large scale rewrites are even less likely to happen once you've got "working" code.
While we all know that assumptions can be bad, the danger is when we don't realize we are making assumptions. In the first story I thought I know what enumerations did. The developers in the second story had used Java Serialization on many projects and thought they fully understood it. In the third story the benefits of XML were considered without awareness of all of the limitations and restrictions that came along. i.e. We didn't know what we didn't know.
The conservative solution to this problem is to only use technologies you have already used successfully on all your projects. While this seems to be favored by many people, I am too much a fan of shiny objects and so like to try new things. The key to using new technologies (or existing ones in new ways) is to limit the scope until you have used it successfully. If it is going to be integral to your design, create a small throw-away prototype first to make sure you find the dark corners and sharp edges first.
The most important solution, though, is to be willing to admit you were wrong. Its impossible to get very far in life without making assumptions, and despite your best precautions, sooner or later, you will be wrong about an assumption. Rather than try to patch on hack after hack to try to make a bad assumption viable, be willing to break from the past and rewrite/redesign code given your new knowledge. Even if it means throwing away a lot of work.
A forum to discuss thoughts, experiences, problems, complaints, questions, and findings in the field of software engineering.
Monday, September 26, 2011
Monday, September 19, 2011
RSpec Lessons Learned
Previously I showed some unit tests written in RSpec. Those test were fairly ugly, partially because I didn't really understand RSpec. In an effort to have better unit tests, I have learned a little more about RSpec.
Read the Latest Documentation
You'd think this would be obvious, but given the lack of links to the documentation on the various "here's advice on using RSpec" blogs, it must not be done a lot. Worse, somehow I had been only looking at the old documentation for version 1 of RSpec. So, if you are going to use RSpec, I suggest you read latest documentation, which as of when I am writing this is RSpec 2.6. This will definitely be helpful.
Make Tests Self Documenting
Each test (it or specify block) can take a description. Sometimes this is necessary, but any time you have documentation (and that's what this description is), you risk having the documentation be out of sync with the code. While not everyone agrees, I am a fan of letting the tests document themselves, whenever possible.
Make Use of Context
Use the "context" keyword to describe what you are doing in a "before" block to set up the state necessary for testing.
Make Use of Describe
Use the "describe" keyword to describe either the noun that is being tested, or the actions that are under test. i.e. if you have actions in a "before" block that are the actions being tested, you should you use "describe" rather than "context"
Make Use of Subject
If you have multiple tests on a single object, make it the RSpec Subject and put it in a describes block. This way, all of your "should" comparisons will be implicitly on this object.
Use Its
Often times you want to test the properties of an object. You can use the "its" method to have the implicit subject of "should" comparisons be the result of the method or array dereference specified.
Let and Subject are Lazy Loaded
The "variables" defined in "let" calls and the "subject" aren't actually evaluated until they are used. So if you never reference a variable specified in a "let", then that code is never executed. This also means that order isn't important. i.e. the following will work:
I actually haven't found this documented, so I may be doing something wrong. But I have found that if I have two different rspec files that each have a "shared_examples_for 'test this object'", this causes problems. I can test each rspec file in isolation and it is fine. But if I try to test both at the same time, I get an error saying that a shared example already exists with the name "test this object".
There are two different solutions to this problem that I have found. If the shared examples are the same, pull them into a common Module that is included. Note, this is better than having repeated code anyway. If the examples are different, then you have to be more unique with the names of the shared examples.
Final Thoughts
It seems that the approach of RSpec is to write tests such that they are self descriptive and so that each test tests exactly one thing. While in theory, this sounds good, I am finding that this results in very verbose test files. That's even with the cleaning up that I have done after learning RSpec better. i.e. I like a lot of what RSpec does, but I am not convinced that it is the best way to write unit tests.
Rewritten Tests
Using what I have learned, I have rewritten the unit tests from before. Here is what they look like now:
Below is what the output looks like. As you can see, if you read it, it describes the tests that are being run more clearly than the old version of the tests.
Read the Latest Documentation
You'd think this would be obvious, but given the lack of links to the documentation on the various "here's advice on using RSpec" blogs, it must not be done a lot. Worse, somehow I had been only looking at the old documentation for version 1 of RSpec. So, if you are going to use RSpec, I suggest you read latest documentation, which as of when I am writing this is RSpec 2.6. This will definitely be helpful.
Make Tests Self Documenting
Each test (it or specify block) can take a description. Sometimes this is necessary, but any time you have documentation (and that's what this description is), you risk having the documentation be out of sync with the code. While not everyone agrees, I am a fan of letting the tests document themselves, whenever possible.
Make Use of Context
Use the "context" keyword to describe what you are doing in a "before" block to set up the state necessary for testing.
describe Thing do let(:thing) {Thing.new} subject {thing} context "empty object" do #insert tests end context "with inherited object" do let(:base) {Thing.new} before(:each) do base["foo"] = 5 thing.prototype = base thing["baz"] = 15 end #insert tests end end
Use the "describe" keyword to describe either the noun that is being tested, or the actions that are under test. i.e. if you have actions in a "before" block that are the actions being tested, you should you use "describe" rather than "context"
describe Thing do let(:thing) {Thing.new} subject {thing} context "empty object" do describe "when assigning via properties" do before(:each) do thing["foo"] = 5 thing["bar"] = "hello" end # insert checks end end end
If you have multiple tests on a single object, make it the RSpec Subject and put it in a describes block. This way, all of your "should" comparisons will be implicitly on this object.
describe Thing do let(:thing) {Thing.new} subject {thing} context "empty object" do describe "keys" do subject {thing.keys} it {should_not be_nil} it {should be_empty} end end end
Often times you want to test the properties of an object. You can use the "its" method to have the implicit subject of "should" comparisons be the result of the method or array dereference specified.
describe Thing do let(:thing) {Thing.new} subject {thing} context "empty object" do its(['newProperty']) {should be_nil} its('newMethod') {should be_nil} end end
The "variables" defined in "let" calls and the "subject" aren't actually evaluated until they are used. So if you never reference a variable specified in a "let", then that code is never executed. This also means that order isn't important. i.e. the following will work:
let (:a) {b + 1} let (:b) {5} specify("show using let") {a.should == (b+1)}Shared_examples and shared_contexts are Global
I actually haven't found this documented, so I may be doing something wrong. But I have found that if I have two different rspec files that each have a "shared_examples_for 'test this object'", this causes problems. I can test each rspec file in isolation and it is fine. But if I try to test both at the same time, I get an error saying that a shared example already exists with the name "test this object".
There are two different solutions to this problem that I have found. If the shared examples are the same, pull them into a common Module that is included. Note, this is better than having repeated code anyway. If the examples are different, then you have to be more unique with the names of the shared examples.
Final Thoughts
It seems that the approach of RSpec is to write tests such that they are self descriptive and so that each test tests exactly one thing. While in theory, this sounds good, I am finding that this results in very verbose test files. That's even with the cleaning up that I have done after learning RSpec better. i.e. I like a lot of what RSpec does, but I am not convinced that it is the best way to write unit tests.
Rewritten Tests
Using what I have learned, I have rewritten the unit tests from before. Here is what they look like now:
require 'spec_helper' describe Thing do let(:thing) {Thing.new} subject {thing} shared_examples_for "simple object" do |map, self_keys| describe "keys" do subject {thing.keys} it {should have(map.size).items} it {should include(*(map.keys))} it {should_not include("noSuchProperty")} end describe "self_keys" do subject {thing.self_keys} before(:each) { self_keys ||= map.keys} it {should have(self_keys.size).items} it {should include(*self_keys)} it {should_not include("noSuchProperty")} end describe "fields" do map.each do |k, v| its([k]) {should == v} end its(['noSuchProperty']) {should be_nil} end describe "methods" do map.each do |k, v| its(k) {should == v} end its('noSuchMethod') {should be_nil} end describe "to_hash" do subject {thing.to_hash} it {should_not be_nil} it {should have(map.size).items} it {should include(*map.keys)} it {should include(map)} end end context "empty object" do describe "keys" do subject {thing.keys} it {should_not be_nil} it {should be_empty} end its(['newProperty']) {should be_nil} its('newMethod') {should be_nil} describe "to_hash" do subject {thing.to_hash} it {should_not be_nil} it {should have(0).items} end describe "when assigning via properties" do before(:each) do thing["foo"] = 5 thing["bar"] = "hello" end it_should_behave_like "simple object", 'foo'=>5, 'bar'=>"hello" end describe "when assigning via methods" do before(:each) do thing.foo = 5 thing.bar = "hello" end it_should_behave_like "simple object", 'foo'=>5, 'bar'=>"hello" end end context "with inherited object" do let(:base) {Thing.new} before(:each) do base["foo"] = 5 base["bar"] = "hello" thing.prototype = base thing["baz"] = 15 thing["bye"] = "bye" end it_should_behave_like "simple object", {'foo'=>5, 'bar'=>"hello", 'baz'=>15, 'bye'=>"bye"}, ['baz', 'bye'] describe "when overriding values" do before(:each) do thing["foo"] = 25 thing["bar"] = "hola" end it_should_behave_like "simple object", 'foo'=>25, 'bar'=>"hola", 'baz'=>15, 'bye'=>"bye" end end end
Below is what the output looks like. As you can see, if you read it, it describes the tests that are being run more clearly than the old version of the tests.
$ rspec spec/models/thing_spec.rb Thing empty object keys should not be nil should be empty ["newProperty"] should be nil newMethod should be nil to_hash should not be nil should have 0 items when assigning via properties it should behave like simple object keys should have 2 items should include "foo" and "bar" should not include "noSuchProperty" self_keys should have 2 items should include "foo" and "bar" should not include "noSuchProperty" fields ["foo"] should == 5 ["bar"] should == "hello" ["noSuchProperty"] should be nil methods foo should == 5 bar should == "hello" noSuchMethod should be nil to_hash should not be nil should have 2 items should include "foo" and "bar" should include {"foo"=>5, "bar"=>"hello"} when assigning via methods it should behave like simple object keys should have 2 items should include "foo" and "bar" should not include "noSuchProperty" self_keys should have 2 items should include "foo" and "bar" should not include "noSuchProperty" fields ["foo"] should == 5 ["bar"] should == "hello" ["noSuchProperty"] should be nil methods foo should == 5 bar should == "hello" noSuchMethod should be nil to_hash should not be nil should have 2 items should include "foo" and "bar" should include {"foo"=>5, "bar"=>"hello"} with inherited object it should behave like simple object keys should have 4 items should include "foo", "bar", "baz", and "bye" should not include "noSuchProperty" self_keys should have 2 items should include "baz" and "bye" should not include "noSuchProperty" fields ["foo"] should == 5 ["bar"] should == "hello" ["baz"] should == 15 ["bye"] should == "bye" ["noSuchProperty"] should be nil methods foo should == 5 bar should == "hello" baz should == 15 bye should == "bye" noSuchMethod should be nil to_hash should not be nil should have 4 items should include "foo", "bar", "baz", and "bye" should include {"foo"=>5, "bar"=>"hello", "baz"=>15, "bye"=>"bye"} when overriding values it should behave like simple object keys should have 4 items should include "foo", "bar", "baz", and "bye" should not include "noSuchProperty" self_keys should have 4 items should include "foo", "bar", "baz", and "bye" should not include "noSuchProperty" fields ["foo"] should == 25 ["bar"] should == "hola" ["baz"] should == 15 ["bye"] should == "bye" ["noSuchProperty"] should be nil methods foo should == 25 bar should == "hola" baz should == 15 bye should == "bye" noSuchMethod should be nil to_hash should not be nil should have 4 items should include "foo", "bar", "baz", and "bye" should include {"foo"=>25, "bar"=>"hola", "baz"=>15, "bye"=>"bye"} Finished in 0.0831 seconds 78 examples, 0 failures
Monday, September 12, 2011
Birth of a Bug
This is a a story of a bug. It was a simple bug. Like many simple problems, on the surface it had a simple cause. Also like many simple problems, in reality it had many contributing factors. Before I tell you about the bug, let me tell you about the application.
let people configure which emails they getAt my place of employment we have various electronic forms that need to be signed by people. For example, if you want to purchase something, there are people who need to approve that purchase - probably your supervisor plus whoever has signature authority for the project you are billing the purchase to. To keep the process flowing people get emails letting them know when they have a form to sign. As it turns out, not everyone wants these emails, and sometimes people only want the emails under certain circumstances. So I wrote an app to let people configure which emails they get.
I deployed this app the other day after work, and promptly the next morning people started using it to turn off email notifications. Shortly thereafter they started sending in complaints that they were still receiving email.
Why didn't it work? How did such a fundamental bug get into the application?
Before answering that question, I'll tell you what the bug was. One of the features of the configuration is that people could turn off email notification, but add a financial exception. As an example, some people only want to get email notifications for requests that are for more than $10,000. Somewhere in the user's NotificationPreferences object is a line that looks similar to:
let people configure which emails they getAt my place of employment we have various electronic forms that need to be signed by people. For example, if you want to purchase something, there are people who need to approve that purchase - probably your supervisor plus whoever has signature authority for the project you are billing the purchase to. To keep the process flowing people get emails letting them know when they have a form to sign. As it turns out, not everyone wants these emails, and sometimes people only want the emails under certain circumstances. So I wrote an app to let people configure which emails they get.
I deployed this app the other day after work, and promptly the next morning people started using it to turn off email notifications. Shortly thereafter they started sending in complaints that they were still receiving email.
Why didn't it work? How did such a fundamental bug get into the application?
Before answering that question, I'll tell you what the bug was. One of the features of the configuration is that people could turn off email notification, but add a financial exception. As an example, some people only want to get email notifications for requests that are for more than $10,000. Somewhere in the user's NotificationPreferences object is a line that looks similar to:
boolean checkFinancialOverride(Double amount) { return (amount >= mMoney); }
where amount is the amount the form is for ($53.24 stapler), and mMoney is the threshold the user set ($10,000). There's a catch though - what is mMoney if the user didn't specify a threshold? Turns out that there are two possible values, depending on how the value got set. If they never have set any preferences at all, this value is NULL, but if they have set a preference for configuration, but never set a threshold, this value gets set to 0. This means that we do NOT want to send a financial override if the mMoney value is either NULL or 0. The actual code looked like:
Don't Make Last Minute Changes
The plan was to deploy the code at 5PM. At 3PM, I had a coworker who suggested that "amount" was a better name for the database column for the threshold than "money" because "amount" is consistent with other projects. (and, of course, "mAmount" would be the Java variable containing the threshold value.)
Anytime you make a change, the risk of breaking something is non-zero. Make the change at the last minute and you won't have time to exhaustively test it. As you have figured out, we broke the code. Then in my "smoke testing" I only chose examples that didn't trigger the new bug. Which is how I unknowingly release code with this bug.
This wasn't a critical user bug. Heck, this wasn't anything that was going to affect the user in any way, shape, or form. I never should've agreed to make this change.
Beware of Semantic Code Changes
When making this last minute name change from "money" to "amount" we split the work up between me and the developer who recommended the change, even though all the code involving this had been written by me. This means that he was changing code that he wasn't familiar with. Here is what the updated code looked like:
Understand Your Changes
Ever run across code in a working system that just can't be right? Before just assuming that the bug is being hidden by other functionality and going ahead and fixing the code, make sure you understand exactly what is happening. If you have access to the original developer (in this case I was just across the hall, a slightly raised voice away), ask why the code is the way it is. It might be a bug, but you might also not be understanding it.
Leverage Refactoring Tools
If you are just renaming a variable, let your IDE rename it for you. This is the type of change that can almost always be done automatically by tools. Use those tools.
Check Your Changes Before Committing
This is something I am a big enough believer in that I may turn it into its own blog post at some point. After making code changes, no matter how large or how trivial, run diff on the code before checking it in. Verify that every change that you are committing is one that you mean. This serves as a simple and quick code review, as well as ensuring that you don't accidentally commit left in debugging statements. Not to mention catching those places where you accidentally inserted or deleted code because of careless clicks.
Run Your Unit Tests
After making any changes, no matter how trivial, make sure you rerun your unit tests. Especially before deploying. You do have unit tests, don't you? Ok, I'll admit it, we had no unit tests on this project, and this is exactly the type of bug that would've gotten caught by unit tests.
Write Understandable Code
While you can follow all the best practices under the sun, you can't force your coworkers to do the same. The best thing you can do to defend against that is to write code that is not only correct but understandable and obvious. Let's look at that query one more time:
Conclusion
Write clean code. Don't make last minute changes. Understand your changes. Write unit tests and use them. These are the steps to avoid unexpected and avoidable bugs.
boolean checkFinancialOverride(Double amount) { return mMoney != null && amount != null && mMoney > 0 && amount >= mMoney; }Don't bother looking for the bug in that code, as the bug isn't there. At least not yet. It didn't show up until deployment day. While the bug was simple, there were a number of "best practices" that were violated that led to the birth of this bug.
Don't Make Last Minute Changes
The plan was to deploy the code at 5PM. At 3PM, I had a coworker who suggested that "amount" was a better name for the database column for the threshold than "money" because "amount" is consistent with other projects. (and, of course, "mAmount" would be the Java variable containing the threshold value.)
This wasn't a critical user bug. Heck, this wasn't anything that was going to affect the user in any way, shape, or form. I never should've agreed to make this change.
Beware of Semantic Code Changes
When making this last minute name change from "money" to "amount" we split the work up between me and the developer who recommended the change, even though all the code involving this had been written by me. This means that he was changing code that he wasn't familiar with. Here is what the updated code looked like:
boolean checkFinancialOverride(Double amount) { return mAmount != null && amount != null && amount >= mAmount; }As you can see, the condition for checking if "mAmount > 0" got dropped. Why was it dropped? I haven't asked, so I don't know. I don't know if it was accidental or intentional. Either way, there are related best practices that should have been followed.
Understand Your Changes
Ever run across code in a working system that just can't be right? Before just assuming that the bug is being hidden by other functionality and going ahead and fixing the code, make sure you understand exactly what is happening. If you have access to the original developer (in this case I was just across the hall, a slightly raised voice away), ask why the code is the way it is. It might be a bug, but you might also not be understanding it.
Leverage Refactoring Tools
If you are just renaming a variable, let your IDE rename it for you. This is the type of change that can almost always be done automatically by tools. Use those tools.
Check Your Changes Before Committing
This is something I am a big enough believer in that I may turn it into its own blog post at some point. After making code changes, no matter how large or how trivial, run diff on the code before checking it in. Verify that every change that you are committing is one that you mean. This serves as a simple and quick code review, as well as ensuring that you don't accidentally commit left in debugging statements. Not to mention catching those places where you accidentally inserted or deleted code because of careless clicks.
Run Your Unit Tests
After making any changes, no matter how trivial, make sure you rerun your unit tests. Especially before deploying. You do have unit tests, don't you? Ok, I'll admit it, we had no unit tests on this project, and this is exactly the type of bug that would've gotten caught by unit tests.
Write Understandable Code
While you can follow all the best practices under the sun, you can't force your coworkers to do the same. The best thing you can do to defend against that is to write code that is not only correct but understandable and obvious. Let's look at that query one more time:
boolean checkFinancialOverride(Double amount) { return mMoney != null && amount != null && mMoney > 0 && amount >= mMoney; }Checking for null is fairly obvious as it prevents NullPointerExceptions when the autounboxing happens. The comparison "amount >= mMoney" is the comparison that is needed. But what's with the "mMoney > 0"? If I hadn't explained it above, would you have guessed why it is here? Maybe, maybe not, either way I would argue that this is a piece of non-obvious code. I will offer up two possible suggestions for how this code could be made more obvious. The first is to add a comment:
boolean checkFinancialOverride(Double amount) { // if mMoney <= 0, it is not a valid value and should be ignored return mMoney != null && amount != null && mMoney > 0 && amount >= mMoney; }But since I think comments are often a sign of confusing code, my other suggestion involves changing the code:
boolean checkFinancialOverride(Double amount) { return isThresholdValid() && amount != null && amount >= mMoney; } boolean isThresholdValid() { return mMoney != null && mMoney > 0 }
Conclusion
Write clean code. Don't make last minute changes. Understand your changes. Write unit tests and use them. These are the steps to avoid unexpected and avoidable bugs.
Monday, September 5, 2011
Unit Testing a Simple Property Model
Previously I showed the beginning of a Ruby implementation of a simple property model. Well, I decided to create some unit tests to help verify that my property model correctly handles properties, especially with the prototype delegation.
RSpec seems to be popular in the rails world, so I used rspec to write my tests. This exercise provided me with examples of the good, the bad, and the ugly with unit tests. The good was straightforward. I don't have an application yet, just this simple library and unit tests provided an easy way to test it. Even better, at one point I changed an implementation detail that required some rewriting of the property model, and by rerunning the tests, I had confidence that I didn't break anything.
The ugly? This is definitely a good example of ugly. I have the unit testing code at the bottom of this post. I found I had a lot of duplication, so I created "shared_examples_for" to reduce redundancy. However, the resulting test code is still really long and while the shared examples helps with sharing tests, I am not sure they help with readability. Now, as Trevor commented, its possible (ok, likely) that the unit tests are ugly because they weren't written well. I will admit to being a newbie at RSpec. I am sure that they could be written better, but I am not convinced they wouldn't still be somewhat ugly.
The bad? Well, there isn't too much bad, yet. It did take time to create the tests, but I had to validate my code somehow. And it is possibly (likely?) that my tests aren't as thorough as they should be, so maybe I have some false confidence in the correctness of my code. But all in all, despite what I said last paragraph, this wasn't that good of an example of the bad. At least not yet. The bad will come when I spend time trying to clean up these tests, rather than using that time to write new code.
And without further ado, here are these unit tests.
And here is the output of running the above tests:
RSpec seems to be popular in the rails world, so I used rspec to write my tests. This exercise provided me with examples of the good, the bad, and the ugly with unit tests. The good was straightforward. I don't have an application yet, just this simple library and unit tests provided an easy way to test it. Even better, at one point I changed an implementation detail that required some rewriting of the property model, and by rerunning the tests, I had confidence that I didn't break anything.
The ugly? This is definitely a good example of ugly. I have the unit testing code at the bottom of this post. I found I had a lot of duplication, so I created "shared_examples_for" to reduce redundancy. However, the resulting test code is still really long and while the shared examples helps with sharing tests, I am not sure they help with readability. Now, as Trevor commented, its possible (ok, likely) that the unit tests are ugly because they weren't written well. I will admit to being a newbie at RSpec. I am sure that they could be written better, but I am not convinced they wouldn't still be somewhat ugly.
The bad? Well, there isn't too much bad, yet. It did take time to create the tests, but I had to validate my code somehow. And it is possibly (likely?) that my tests aren't as thorough as they should be, so maybe I have some false confidence in the correctness of my code. But all in all, despite what I said last paragraph, this wasn't that good of an example of the bad. At least not yet. The bad will come when I spend time trying to clean up these tests, rather than using that time to write new code.
And without further ado, here are these unit tests.
require 'spec_helper' describe Thing do let(:foo) {5} let(:bar) {"hello"} shared_examples_for "simple properties" do describe "keys" do it {thing.keys.size.should == 2} end describe "foo" do it {thing['foo'].should == foo} it {thing.foo.should == foo} end describe "bar" do it {thing['bar'].should == bar} it {thing.bar.should == bar} end describe "baz" do it {thing['baz'].should be_nil} it {thing.baz.should be_nil} end describe "to_hash" do it {thing.to_hash.should_not be_nil} it {thing.to_hash.size.should == 2} it {thing.to_hash['foo'].should == foo} it {thing.to_hash['bar'].should == bar} end end shared_examples_for "nested properties" do describe "keys" do it {thing.keys.size.should == 4} it {thing.self_keys.size.should == 2} it {thing.self_keys.include?('b2').should be_true} it {thing.self_keys.include?('bye').should be_true} it {thing.self_keys.include?('bar').should be_false} end describe "foo" do it {thing['foo'].should == 5} it {thing.foo.should == 5} end describe "bar" do it {thing['bar'].should == "hello"} it {thing.bar.should == "hello"} end describe "b2" do it {thing['b2'].should == 15} it {thing.b2.should == 15} end describe "bye" do it {thing['bye'].should == "bye"} it {thing.bye.should == "bye"} end describe "baz" do it {thing['baz'].should be_nil} it {thing.baz.should be_nil} end describe "to_hash" do it {thing.to_hash.should_not be_nil} it {thing.to_hash.size.should == 4} it {thing.to_hash['foo'].should == 5} it {thing.to_hash['bar'].should == "hello"} it {thing.to_hash['b2'].should == 15} it {thing.to_hash['bye'].should == "bye"} end end describe "no inheritance" do let(:thing) {Thing.new} subject {thing} describe "new object" do describe "keys" do it {thing.keys.should_not be_nil} it {thing.keys.should be_empty} end describe "properties" do it {thing['foo'].should be_nil} it {thing.foo.should be_nil} end describe "to_hash" do it {thing.to_hash.should_not be_nil} it {thing.to_hash.size.should == 0} end end describe "adding properties to object" do before(:each) do thing['foo'] = 5 thing['bar'] = "hello" end it_should_behave_like "simple properties" it {thing.self_keys.size.should == 2} it {thing.self_keys.include?('foo').should be_true} it {thing.self_keys.include?('bar').should be_true} it {thing.self_keys.include?('baz').should be_false} end describe "adding fields to object" do before(:each) do thing.foo = 5 thing.bar = "hello" end it_should_behave_like "simple properties" it {thing.self_keys.size.should == 2} it {thing.self_keys.include?('foo').should be_true} it {thing.self_keys.include?('bar').should be_true} it {thing.self_keys.include?('baz').should be_false} end end describe "inheritance" do let(:thing) do base = Thing.new base.foo = 5 base['bar'] = 'hello' foo = Thing.new foo.prototype = base foo end subject {thing} describe "new object" do it_should_behave_like "simple properties" it {thing.self_keys.include?('bar').should be_false} it {thing.self_keys.include?('baz').should be_false} end describe "adding properties to object" do before(:each) do thing['b2'] = 15 thing['bye'] = "bye" end it_should_behave_like "nested properties" end describe "adding fields to object" do before(:each) do thing.b2 = 15 thing.bye = "bye" end it_should_behave_like "nested properties" end describe "adding redundant fields to object" do let(:foo) {15} let(:bar) {"bar2"} before(:each) do thing.foo = foo thing.bar = bar end it_should_behave_like "simple properties" it {thing.self_keys.size.should == 2} it {thing.self_keys.include?('foo').should be_true} it {thing.self_keys.include?('bar').should be_true} it {thing.self_keys.include?('baz').should be_false} end end end
And here is the output of running the above tests:
$ rspec spec/models/thing_spec.rb Thing no inheritance new object keys should not be nil should be empty properties should be nil should be nil to_hash should not be nil should == 0 adding properties to object should == 2 should be true should be true should be false it should behave like simple properties keys should == 2 foo should == 5 should == 5 bar should == "hello" should == "hello" baz should be nil should be nil to_hash should not be nil should == 2 should == 5 should == "hello" adding fields to object should == 2 should be true should be true should be false it should behave like simple properties keys should == 2 foo should == 5 should == 5 bar should == "hello" should == "hello" baz should be nil should be nil to_hash should not be nil should == 2 should == 5 should == "hello" inheritance new object should be false should be false it should behave like simple properties keys should == 2 foo should == 5 should == 5 bar should == "hello" should == "hello" baz should be nil should be nil to_hash should not be nil should == 2 should == 5 should == "hello" adding properties to object it should behave like nested properties keys should == 4 should == 2 should be true should be true should be false foo should == 5 should == 5 bar should == "hello" should == "hello" b2 should == 15 should == 15 bye should == "bye" should == "bye" baz should be nil should be nil to_hash should not be nil should == 4 should == 5 should == "hello" should == 15 should == "bye" adding fields to object it should behave like nested properties keys should == 4 should == 2 should be true should be true should be false foo should == 5 should == 5 bar should == "hello" should == "hello" b2 should == 15 should == 15 bye should == "bye" should == "bye" baz should be nil should be nil to_hash should not be nil should == 4 should == 5 should == "hello" should == 15 should == "bye" adding redundant fields to object should == 2 should be true should be true should be false it should behave like simple properties keys should == 2 foo should == 15 should == 15 bar should == "bar2" should == "bar2" baz should be nil should be nil to_hash should not be nil should == 2 should == 15 should == "bar2" Finished in 0.06217 seconds 106 examples, 0 failures
Subscribe to:
Posts (Atom)