From b7347dcc445fe1b3a61a79152c09c0d52fc4ea28 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 19 May 2017 20:27:25 +0200 Subject: [PATCH] Refactor `CLI::Cleanup`. --- Library/Homebrew/cask/lib/hbc/cli/cleanup.rb | 22 ++--- Library/Homebrew/cask/lib/hbc/cli/doctor.rb | 2 +- .../Homebrew/test/cask/cli/cleanup_spec.rb | 90 ++++++++----------- 3 files changed, 43 insertions(+), 71 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb b/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb index 9ebccabd02..a62b8d069a 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/cleanup.rb @@ -13,29 +13,19 @@ module Hbc end def self.run(*args) - if args.empty? - default.cleanup! - else - default.cleanup(args) - end - end - - def self.default - @default ||= new(Hbc.cache, CLI.outdated?) + new(*args).run end attr_reader :cache_location, :outdated_only - def initialize(cache_location, outdated_only) + + def initialize(*args, cache_location: Hbc.cache, outdated_only: CLI.outdated?) + @args = args @cache_location = Pathname.new(cache_location) @outdated_only = outdated_only end - def cleanup! - remove_cache_files - end - - def cleanup(tokens) - remove_cache_files(*tokens) + def run + remove_cache_files(*@args) end def cache_files diff --git a/Library/Homebrew/cask/lib/hbc/cli/doctor.rb b/Library/Homebrew/cask/lib/hbc/cli/doctor.rb index 031f788247..11917ae6b7 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/doctor.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/doctor.rb @@ -107,7 +107,7 @@ module Hbc end def self.render_cached_downloads - cleanup = CLI::Cleanup.default + cleanup = CLI::Cleanup.new count = cleanup.cache_files.count size = cleanup.disk_cleanup_size msg = user_tilde(Hbc.cache.to_s) diff --git a/Library/Homebrew/test/cask/cli/cleanup_spec.rb b/Library/Homebrew/test/cask/cli/cleanup_spec.rb index c7ef356c04..037fa1bd40 100644 --- a/Library/Homebrew/test/cask/cli/cleanup_spec.rb +++ b/Library/Homebrew/test/cask/cli/cleanup_spec.rb @@ -1,19 +1,20 @@ describe Hbc::CLI::Cleanup, :cask do let(:cache_location) { Pathname.new(Dir.mktmpdir).realpath } - let(:cleanup_outdated) { false } + let(:outdated_only) { false } - subject { described_class.new(cache_location, cleanup_outdated) } + subject { described_class.new(*cask_tokens, cache_location: cache_location, outdated_only: outdated_only) } after do cache_location.rmtree end describe "cleanup" do - it "removes cached downloads of given casks" do - cleaned_up_cached_download = "caffeine" + let(:cask_token) { "caffeine" } + let(:cask_tokens) { [cask_token] } + it "removes cached downloads of given casks" do cached_downloads = [ - cache_location.join("#{cleaned_up_cached_download}--latest.zip"), + cache_location.join("#{cask_token}--latest.zip"), cache_location.join("transmission--2.61.dmg"), ] @@ -22,9 +23,9 @@ describe Hbc::CLI::Cleanup, :cask do cleanup_size = cached_downloads[0].disk_usage expect { - subject.cleanup(cleaned_up_cached_download) + subject.run }.to output(<<-EOS.undent).to_stdout - ==> Removing cached downloads for #{cleaned_up_cached_download} + ==> Removing cached downloads for #{cask_token} #{cached_downloads[0]} ==> This operation has freed approximately #{disk_usage_readable(cleanup_size)} of disk space. EOS @@ -32,61 +33,42 @@ describe Hbc::CLI::Cleanup, :cask do expect(cached_downloads[0].exist?).to eq(false) expect(cached_downloads[1].exist?).to eq(true) end - end - describe "cleanup!" do - it "removes cached downloads" do - cached_download = cache_location.join("SomeDownload.dmg") - FileUtils.touch(cached_download) - cleanup_size = subject.disk_cleanup_size + context "when no argument is given" do + let(:cask_tokens) { [] } - expect { - subject.cleanup! - }.to output(<<-EOS.undent).to_stdout - ==> Removing cached downloads - #{cached_download} - ==> This operation has freed approximately #{disk_usage_readable(cleanup_size)} of disk space. - EOS - - expect(cached_download.exist?).to eq(false) - end - - # TODO: uncomment when unflaky. - # it "does not removed locked files" do - # cached_download = cache_location.join("SomeDownload.dmg") - # FileUtils.touch(cached_download) - # cleanup_size = subject.disk_cleanup_size - # - # File.new(cached_download).flock(File::LOCK_EX) - # - # expect(Hbc::Utils).to be_file_locked(cached_download) - # - # expect { - # subject.cleanup! - # }.to output(<<-EOS.undent).to_stdout - # ==> Removing cached downloads - # skipping: #{cached_download} is locked - # ==> This operation has freed approximately #{disk_usage_readable(cleanup_size)} of disk space. - # EOS - # - # expect(cached_download.exist?).to eq(true) - # end - - context "when cleanup_outdated is specified" do - let(:cleanup_outdated) { true } - - it "does not remove cache files newer than 10 days old" do - cached_download = cache_location.join("SomeNewDownload.dmg") + it "removes all cached downloads" do + cached_download = cache_location.join("SomeDownload.dmg") FileUtils.touch(cached_download) + cleanup_size = subject.disk_cleanup_size expect { - subject.cleanup! + subject.run }.to output(<<-EOS.undent).to_stdout - ==> Removing cached downloads older than 10 days old - Nothing to do + ==> Removing cached downloads + #{cached_download} + ==> This operation has freed approximately #{disk_usage_readable(cleanup_size)} of disk space. EOS - expect(cached_download.exist?).to eq(true) + expect(cached_download.exist?).to eq(false) + end + + context "and :outdated_only is specified" do + let(:outdated_only) { true } + + it "does not remove cache files newer than 10 days old" do + cached_download = cache_location.join("SomeNewDownload.dmg") + FileUtils.touch(cached_download) + + expect { + subject.run + }.to output(<<-EOS.undent).to_stdout + ==> Removing cached downloads older than 10 days old + Nothing to do + EOS + + expect(cached_download.exist?).to eq(true) + end end end end