Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ChefSpec Can Be Faster #275

Closed
DracoAter opened this issue Dec 12, 2013 · 8 comments
Closed

ChefSpec Can Be Faster #275

DracoAter opened this issue Dec 12, 2013 · 8 comments

Comments

@DracoAter
Copy link
Contributor

Hi,

I came across the problem that my chefspec run ~20 minutes, because I have more than 800 examples to run.

Chefspec proposes to use let block to run converging. That means it is run before every example. But the big difference between chefspec tests and other rspec tests
is that this converging operation to create a Subject under Test (SuT) takes a lot more time than SuT creation in other rspec tests (something like User.new). Another thing is that as long as the chef_run value is obtained, we do not have to change it anywhere, we just run some examples and make sure that some resource was created/updated. So in fact converging before every example is a huge overhead.

So instead we could use before :all to save chef_run value.

describe 'example::default' do
  before :all do
    @chef_run = ChefSpec::Runner.new.converge(described_recipe)
  end
  let(:chef_run) { @chef_run }
  it 'installs foo' do
    expect(chef_run).to install_package('foo')
  end
  it 'installs bar' do
    expect(chef_run).to install_package('bar')
  end
end

This way converging is run only once for every scope of examples.

There is a drawback however: it's impossible to stub/mock anything inside the before :all block, but it's RSpec limitation.

@sethvargo
Copy link
Contributor

@DracoAter you're totally right, but you're also very familiar with RSpec. You should totally do everything you can to improve test speeds. But keep in mind that the majority of ChefSpec users aren't RSpec magicians or even Ruby developers, so I'm trying to provide the smoothest path to adoption. Let me explain a bit further.

There's two kinds of speed:

  • Speed to deliver
  • Speed to run

Using a let block provides the fastest speed to deliver, and a fairly reasonable speed to run. Now, if you take a moment to review the past (say) 50 issues, you'll find that about 70% of them are related to stubbing/mocking LWRPs, libraries, MixlibShellout, etc. Again, most users are not RSpec junkies like you or me, so telling people to use a before block to cache the converge, unless you're mocking/stubbing, then do this other thing - is not an ideal user experience in my opinion.

Additionally, your solution implies a single platform/version being tested, which is mostly not the case for many users. They have to support RHEL, Debian, and OSX (for example). Then users encounter the same paradox I describe here in the README.

Unfortunately, there's nothing I can do "under the hood" to make ChefSpec faster. I've already optimized every possible edge case between 2.x and 3.x 😦. I also don't feel comfortable adding something like this to the README, for fear it will confuse newbies. However, here's the good news - you should totally write a blog post / gist about this. Add a little more educational information about why this is different and how it behaves under the hood, add some benchmarks, etc. Ping me and I'll add a link to it in the README 😄.

There's "yet another pattern" you could use as well, which is the cached subject:

let(:chef_run) {...}

describe chef_run do
  it { should install_package('foo') }
  it { should install_package('bar') }
end

I believe that will only perform one converge per describe block. I'd be interested in seeing that benchmarked as well.

I'll keep this issue open so we can continue discussing and figure out a plan moving forward. I'd also encourage you to hang out in IRC on #chefspec if you have the time. I know you're very active in Stackoverflow-land, and I'd love your help answering questions in IRC 😄.

@ranjib
Copy link
Contributor

ranjib commented Dec 12, 2013

we have 2K+ tests and we hit this issue, and my argument against using let was exact same. we moved to using caching/momozing the runner object. https://github.com/PagerDuty/chef_example/blob/master/spec/spec_helper.rb#L40

@sethvargo
Copy link
Contributor

@ranjib any benchmarks? I know you have written some awesome posts on the PagerDuty blog - wanna whip something up about why you switched to a memorized runner?

@ranjib
Copy link
Contributor

ranjib commented Dec 12, 2013

yeah, it used take close to 27 min, but that was when we had around 1.3K tests. now its close to 2 min.

Note: high number of assertions dont mean high number of specs. we keep most of the recipe related assertions as shared_examples and then we use them to test individual recipes, roles (which uses those recipes), and environments. there are some intermediate recipes that are only used by top level recipes (can not be used by their own), i made them parameterized shared_examples so that we can assert against differing conditions. this leads to quiet a high number of assertions, but we have found it useful to detect attribute override, conditional inclusion, scaffolding etc.

@sethvargo
Copy link
Contributor

@ranjib damn. But still a huge time reduction!

@DracoAter
Copy link
Contributor Author

Wow, great Thanks to both of you, @ranjib and @sethvargo! I know that ChefSpec itself is quite quick, nothing we can do here. I just needed some ideas how to run the specs faster :) Just thought it would be a good place to start conversation. And I will definitely write a blogpost on the topic, when I solve the problem.

@sethvargo
Copy link
Contributor

@DracoAter cool. I'm going to close this, but ping me when you get something together - happy to link 😄

@DracoAter
Copy link
Contributor Author

Hi, I have figured it out. See my blogpost on it. http://dracoater.blogspot.com/2013/12/testing-chef-cookbooks-part-25-speeding.html
Actually, @sethvargo, if you think it's a good idea, you could include this SpecHelper module into Chefspec?!

Once again, thanks.

sethvargo added a commit that referenced this issue Dec 14, 2013
The cacher module allows for ultra-fast tests by caching the results of a
Chef Client Run in memory across an example group. In testing, this can
reduce the total testing time by a factor of 10x. This strategy is _not_ the
default behavior, because it has implications surrounding stubbing and is
_not_ threadsafe!

- Refs: #275
- Refs: http://dracoater.blogspot.com/2013/12/testing-chef-cookbooks-part-25-speeding.html
- Credits: @DracoAter
@lock lock bot locked as resolved and limited conversation to collaborators Aug 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants