Ok, so here''s the scenario. I have a top-level module, with a method called load_file. It should take a file name, get the YAML module (Syck) to parse it, and then send the result to a class, which should convert that result into a series of objects. I''m not sure what specs, if any, I should write for load_file. I can get into the nitty gritty details of exactly what methods it should be calling, but that seems brittle. I can skip all that and just make assertions about the final series of objects, but I''d rather put those specs on the class that''s actually doing the conversion. Suggestions? Here''s the code that I (think I) want to drive out: module BTree def self.load_file file_name raw = YAML::load_file file_name tree = BehaviorTreeCreator.new(raw).create end end -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/rspec-users/attachments/20101030/9be065e9/attachment.html>
On Oct 30, 2010, at 11:40 AM, Andrew Wagner wrote:> Ok, so here''s the scenario. I have a top-level module, with a method called load_file. It should take a file name, get the YAML module (Syck) to parse it, and then send the result to a class, which should convert that result into a series of objects. > > I''m not sure what specs, if any, I should write for load_file. I can get into the nitty gritty details of exactly what methods it should be calling, but that seems brittle. I can skip all that and just make assertions about the final series of objects, but I''d rather put those specs on the class that''s actually doing the conversion. > > Suggestions? Here''s the code that I (think I) want to drive out: > > module BTree > def self.load_file file_name > raw = YAML::load_file file_name > tree = BehaviorTreeCreator.new(raw).create > end > endOne approach would be: describe BTree do describe "::load_file" do let(:filename) { ''/tmp/btree_example.yml'' } let(:content) { "---\na: 1\n" } before { File.open(filename,''w'') {|f| f.write content } } after { File.delete filename } it "reads and parses the content of the file" do BTree.load_file(filename).should eq(BehaviorTreeCreator.new(content).create end end end Assuming there are specs for BehaviorTreeCreator.new(content).create, then this tells the story pretty well without being terribly invasive. WDYT? David
If you''re writing a unit test for a method, the general rule of thumb is to test what the method itself does. That sounds tautological, but it''s meant to define the boundary of what you should and shouldn''t care about. In this case, you only care about the fact that `YAML::load_file` is called, and that `BehaviorTreeCreator.new` is then called with the result of that, and that *that* result has `create` invoked on it. That''s it: if those three things happen, then this method is working as you expect. Your unit tests for BehaviorTreeCreator.new and .create will probably be richer. ~ jf -- John Feminella Principal Consultant, BitsBuilder LI: http://www.linkedin.com/in/fjsquared SO: http://stackoverflow.com/users/75170/
Yeah, that''s about what I figured, thanks. This is what I came up with: require ''spec_helper.rb'' require ''yaml'' describe "BTree" do describe ".load_file" do context "when the file exists" do before(:all) { File.open("test_file.yaml", ''w'') {|f| } } after(:all) { File.delete "test_file.yaml"} after(:each) { BTree.load_file "test_file.yaml" } it "parses the yaml in the file" do YAML.stub!(:load_file).and_return(:foo) YAML.should_receive(:load_file).with("test_file.yaml").and_return(:foo) end it "converts the raw data into a tree" do fake_creator = Object.new fake_creator.stub! :create YAML.stub!(:load_file).and_return(:data) BehaviorCreator.stub!(:new).and_return :tree BehaviorCreator.should_receive(:new).with(:data).and_return fake_creator fake_creator.should_receive :create end end context "when the file doesn''t exist" do it "raises an error" do lambda { BTree.load_file "non_existent.yaml" }.should raise_error end end end end On Sat, Oct 30, 2010 at 12:47 PM, John Feminella <johnf at distb.net> wrote:> If you''re writing a unit test for a method, the general rule of thumb > is to test what the method itself does. That sounds tautological, but > it''s meant to define the boundary of what you should and shouldn''t > care about. > > In this case, you only care about the fact that `YAML::load_file` is > called, and that `BehaviorTreeCreator.new` is then called with the > result of that, and that *that* result has `create` invoked on it. > That''s it: if those three things happen, then this method is working > as you expect. > > Your unit tests for BehaviorTreeCreator.new and .create will probably be > richer. > > ~ jf > -- > John Feminella > Principal Consultant, BitsBuilder > LI: http://www.linkedin.com/in/fjsquared > SO: http://stackoverflow.com/users/75170/ > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/rspec-users/attachments/20101030/0c91b8c3/attachment.html>
On Sat, Oct 30, 2010 at 12:46 PM, David Chelimsky <dchelimsky at gmail.com>wrote:> On Oct 30, 2010, at 11:40 AM, Andrew Wagner wrote: > > > Ok, so here''s the scenario. I have a top-level module, with a method > called load_file. It should take a file name, get the YAML module (Syck) to > parse it, and then send the result to a class, which should convert that > result into a series of objects. > > > > I''m not sure what specs, if any, I should write for load_file. I can get > into the nitty gritty details of exactly what methods it should be calling, > but that seems brittle. I can skip all that and just make assertions about > the final series of objects, but I''d rather put those specs on the class > that''s actually doing the conversion. > > > > Suggestions? Here''s the code that I (think I) want to drive out: > > > > module BTree > > def self.load_file file_name > > raw = YAML::load_file file_name > > tree = BehaviorTreeCreator.new(raw).create > > end > > end > > One approach would be: > > describe BTree do > describe "::load_file" do > let(:filename) { ''/tmp/btree_example.yml'' } > let(:content) { "---\na: 1\n" } > before { File.open(filename,''w'') {|f| f.write content } } > after { File.delete filename } > > it "reads and parses the content of the file" do > BTree.load_file(filename).should > eq(BehaviorTreeCreator.new(content).create > end > end > end > > Assuming there are specs for BehaviorTreeCreator.new(content).create, then > this tells the story pretty well without being terribly invasive. > > WDYT? > > David > > > > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >I like this, though it''s missing a YAML::load(content). I.e., BTree.load_file(filename).should eq(BehaviorTreeCreator.new(YAML::load content).create -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/rspec-users/attachments/20101030/932176a5/attachment.html>
On 30 October 2010 14:40, Andrew Wagner <wagner.andrew at gmail.com> wrote:> Ok, so here''s the scenario. I have a top-level module, with a method called > load_file. It should take a file name, get the YAML module (Syck) to parse > it, and then send the result to a class, which should convert that result > into a series of objects. > > I''m not sure what specs, if any, I should write for load_file. I can get > into the nitty gritty details of exactly what methods it should be calling, > but that seems brittle. I can skip all that and just make assertions about > the final series of objects, but I''d rather put those specs on the class > that''s actually doing the conversion. > > Suggestions? Here''s the code that I (think I) want to drive out: > > module BTree > def self.load_file file_name > raw = YAML::load_file file_name > tree = BehaviorTreeCreator.new(raw).create > end > end > >Its not obvious what specs to write for load_file because load_file is poorly named. It does far more than load a file. If you write your specs for load file and read them, they won''t make sense e.g. load_file should use YAML (why should it use YAML, and what happens if the file isn''t YAML load_file should create a behaviour tree (why should it create a TREE) Perhaps BTree only needs a yaml_to_tree, and you can pass the yaml into the method. If this was the case you might spec the following: what happens if you pass invalid YAML what happens if you pass valid YAML what happens if you pass nothing Generally if a method is hard to spec, then its smelly, there is something wrong with it, it needs work HTH Andrew> _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/rspec-users/attachments/20101031/8ca3af46/attachment.html>
On Sat, Oct 30, 2010 at 9:54 PM, Andrew Premdas <apremdas at gmail.com> wrote:> On 30 October 2010 14:40, Andrew Wagner <wagner.andrew at gmail.com> wrote: > >> Ok, so here''s the scenario. I have a top-level module, with a method >> called load_file. It should take a file name, get the YAML module (Syck) to >> parse it, and then send the result to a class, which should convert that >> result into a series of objects. >> >> I''m not sure what specs, if any, I should write for load_file. I can get >> into the nitty gritty details of exactly what methods it should be calling, >> but that seems brittle. I can skip all that and just make assertions about >> the final series of objects, but I''d rather put those specs on the class >> that''s actually doing the conversion. >> >> Suggestions? Here''s the code that I (think I) want to drive out: >> >> module BTree >> def self.load_file file_name >> raw = YAML::load_file file_name >> tree = BehaviorTreeCreator.new(raw).create >> end >> end >> >> > Its not obvious what specs to write for load_file because load_file is > poorly named. It does far more than load a file. If you write your specs > for load file and read them, they won''t make sense e.g. > > load_file should use YAML (why should it use YAML, and what happens if the > file isn''t YAML > load_file should create a behaviour tree (why should it create a TREE) > > Perhaps BTree only needs a yaml_to_tree, and you can pass the yaml into the > method. If this was the case you might spec the following: > what happens if you pass invalid YAML > what happens if you pass valid YAML > what happens if you pass nothing > > Generally if a method is hard to spec, then its smelly, there is something > wrong with it, it needs work > > HTH > > Andrew > > >> _______________________________________________ >> rspec-users mailing list >> rspec-users at rubyforge.org >> http://rubyforge.org/mailman/listinfo/rspec-users >> > > > _______________________________________________ > rspec-users mailing list > rspec-users at rubyforge.org > http://rubyforge.org/mailman/listinfo/rspec-users >An interesting perspective. The purpose of the behavior tree module is to provide a simple, standard way to represent behaviors in a plain-text format. I''m borrowing YAML because it seems like a good, lightweight approach. I''m using load_file (and load) to mirror YAML::load_file (and YAML::load). I''ll definitely take some of your ideas into consideration, thanks. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/rspec-users/attachments/20101030/87e79cb4/attachment-0001.html>