I'm not particularly proud of this code, but I don't see an alternative that I like better. Eliminating Puppet::Util::ExecutionStub in favor of a global variable seems very clumsy (not to mention that it would leave no place to put comments warning that it is intended for spec testing only). As for your "guard" suggestion, I'm not seeing the value that it adds, and I'm wary of "at least it's doing something" as an argument for making a change.
On Mon, Mar 7, 2011 at 5:13 PM, Markus Roberts <mar...@puppetlabs.com>wrote: > Am I missing something, or is this: > > > --- /dev/null >> +++ b/lib/puppet/util/execution_stub.rb >> @@ -0,0 +1,26 @@ >> +module Puppet::Util >> + class ExecutionStub >> + class << self >> + # Set a stub block that Puppet::Util.execute() should invoke >> instead >> + # of actually executing commands on the target machine. Intended >> + # for spec testing. >> + # >> + # The arguments passed to the block are |command, options|, where >> + # command is an array of strings and options is an options hash. >> + def set(&block) >> + @value = block >> + end >> + >> + # Uninstall any execution stub, so that calls to >> + # Puppet::Util.execute() behave normally again. >> + def reset >> + @value = nil >> + end >> + >> + # Retrieve the current execution stub, or nil if there is no stub. >> + def current_value >> + @value >> + end >> + end >> + end >> +end >> > > ...just a really complicated & java-ish way to "implement" a global > variable? What does all the machinery buy us? If it's an abstraction > you're wanting, why not something like: > > class module Puppet::Util::ExecutionStub > > # Set a stub block that Puppet::Util.execute() should invoke instead > # of actually executing commands on the target machine. Intended > # for spec testing. > def self.set(&block) > @stub = block > > end > > # Uninstall any execution stub, so that calls to > # Puppet::Util.execute() behave normally again. > def self.reset > @stub = nil > end > > > # > # command is an array of strings and options is an options hash. > def self.guard(command,options,&normal_code) > @stub ? @stub.call(command,options) : yield(command,options) > end > end > > And then use it like: > > Puppet::Util::ExecutionStub.guard(command, arguments) { > ...normal execution code... > } > > I'm still not fond of it, but at least it's doing something. > > -- M > ----------------------------------------------------------- > When in trouble or in doubt, run in circles, > scream and shout. -- 1920's parody of the > maritime general prudential rule > ------------------------------------------------------------ > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To post to this group, send email to puppet-dev@googlegroups.com. > To unsubscribe from this group, send email to > puppet-dev+unsubscr...@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to puppet-dev@googlegroups.com. To unsubscribe from this group, send email to puppet-dev+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.