Skip to content

Commit

Permalink
extract check_fail! from check! in Bin to show warnings only once, re…
Browse files Browse the repository at this point in the history
…solves #69
  • Loading branch information
toy committed Nov 7, 2014
1 parent 8778234 commit 31b0b59
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## unreleased

* Don't warn multiple times about problematic binary [#69](https://github.com/toy/image_optim/issues/69) [@toy](https://github.com/toy)
* Run gisicle two times (with interlace off then with on) if interlace is not set explicitly [#70](https://github.com/toy/image_optim/issues/70) [@toy](https://github.com/toy)
* Remove app and other extensions from gif images [@toy](https://github.com/toy)
* Change behaviour of gifsicle interlace option to deinterlace for `false`, pass `nil` to leave as is [@toy](https://github.com/toy)
Expand Down
4 changes: 3 additions & 1 deletion lib/image_optim/bin_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,12 @@ def resolve!(name)
end

@bins[name] = bin

bin.check! if bin
end

if @bins[name]
@bins[name].check!
@bins[name].check_fail!
else
fail BinNotFound, "`#{name}` not found"
end
Expand Down
20 changes: 16 additions & 4 deletions lib/image_optim/bin_resolver/bin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def to_s
"#{name} #{version || '?'} at #{path}"
end

# Fail or warn if version is known to misbehave depending on severity
def check!
# Fail if version will not work properly
def check_fail!
fail BadVersion, "didn't get version of #{self}" unless version

is = ComparableCondition.is
Expand All @@ -32,15 +32,27 @@ def check!
when c = is.between?('1.7.60', '1.7.65')
fail BadVersion, "#{self} (#{c}) is known to produce broken pngs"
end
when :pngquant
case version
when c = is < '2.0'
fail BadVersion, "#{self} (#{c}) is not supported"
end
end
end

# Run check_fail!, otherwise warn if version is known to misbehave
def check!
check_fail!

is = ComparableCondition.is
case name
when :advpng
case version
when c = is < '1.17'
warn "WARN: #{self} (#{c}) does not use zopfli"
end
when :pngquant
case version
when c = is < '2.0'
fail BadVersion, "#{self} (#{c}) is not supported"
when c = is < '2.1'
warn "WARN: #{self} (#{c}) may be lossy even with quality `100-`"
end
Expand Down
13 changes: 8 additions & 5 deletions spec/image_optim/bin_resolver_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ def self.path
expect(resolver).to receive(:full_path).with(:ls).and_return('/bin/ls')
bin = double
expect(Bin).to receive(:new).with(:ls, '/bin/ls').and_return(bin)
expect(bin).to receive(:check!).exactly(5).times
expect(bin).to receive(:check!).once
expect(bin).to receive(:check_fail!).exactly(5).times

5.times do
expect(resolver.resolve!(:ls)).to eq(bin)
Expand Down Expand Up @@ -149,7 +150,8 @@ def self.path
bin = double
expect(Bin).to receive(:new).
with(:image_optim, File.expand_path(path)).and_return(bin)
expect(bin).to receive(:check!).exactly(5).times
expect(bin).to receive(:check!).once
expect(bin).to receive(:check_fail!).exactly(5).times

at_exit_blocks = []
expect(resolver).to receive(:at_exit).once do |&block|
Expand Down Expand Up @@ -202,17 +204,18 @@ def self.path
bin = double
expect(Bin).to receive(:new).once.with(:ls, '/bin/ls').and_return(bin)

check_count = 0
count = 0
mutex = Mutex.new
allow(bin).to receive(:check!){ mutex.synchronize{ check_count += 1 } }
allow(bin).to receive(:check!).once
allow(bin).to receive(:check_fail!){ mutex.synchronize{ count += 1 } }

10.times.map do
Thread.new do
resolver.resolve!(:ls)
end
end.each(&:join)

expect(check_count).to eq(10)
expect(count).to eq(10)
end
end

Expand Down

0 comments on commit 31b0b59

Please sign in to comment.