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

Check for existing persistent cache files on demand #26

Merged
merged 6 commits into from
Dec 15, 2013

Conversation

dwhelan
Copy link

@dwhelan dwhelan commented Aug 24, 2013

When I was using puffing_billy I noticed that for the first request it did not check for a persisted cache file. Later, I realized that I was not calling Billy.proxy.restore_cache in my config and also did not notice the Billy.cache.load_dir method.

However, I had already written additional logic to check for a persisted cache entry on demand. And I think this may be an improvement for the existing approach for a few reasons:

  • only requested cache files are read thus speeding up tests
  • eliminates the need for Billy.proxy.restore_cache since cache files will be checked if the entry is not already in the memory cache
  • clients do not need to worry about timing the invocation of Billy.proxy.restore_cache thus eliminating the 'race condition' noted in the readme file

@ronwsmith
Copy link
Collaborator

I've been using the changes in this pull request for several weeks and they works great! Thanks for doing this.

Thought I'd share another benefit I got out of lazy-loading the cache files. When authoring tests that dynamically hit external APIs, a lot of extra cache files can be created that you don't want in the codebase.

With the changes in this pull request, I was able to implement the following in my spec_helper.rb to delete any cache files not used when running the full test suite via DELETE_UNUSED_CACHE=true rake spec:features:

  config.after(:suite, type: :feature) do
    if ENV['DELETE_UNUSED_CACHE'] && defined?(Billy)
      puts "\n*** ALL UNUSED CACHE FILES WILL BE DELETED ***"
      local_cache_path = Rails.root.join(Billy.config.cache_path)
      used_cache_files = Billy.proxy.instance_variable_get(:@cache)

      Dir.glob(local_cache_path+'*.yml') do |full_filename|
        filename = full_filename.gsub(local_cache_path.to_s, '')
        unless used_cache_files[filename.gsub('.yml','')]
          puts " - DELETING #{filename}"
          File.delete(local_cache_path+filename)
        end
      end
    end
  end

@dwhelan
Copy link
Author

dwhelan commented Dec 5, 2013

@ronwsmith I'm glad you found the pull request useful. I like your deletion logic and I will try that out.

oesmith added a commit that referenced this pull request Dec 15, 2013
Check for existing persistent cache files on demand
@oesmith oesmith merged commit b0d9ec8 into oesmith:master Dec 15, 2013
@oesmith
Copy link
Owner

oesmith commented Dec 15, 2013

Merged, thanks!

@dwhelan
Copy link
Author

dwhelan commented Dec 16, 2013

Thanks for merging this in!

@dwhelan dwhelan deleted the persistence branch December 16, 2013 20:04
@dwhelan dwhelan restored the persistence branch December 16, 2013 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants