Merge pull request #4823 from reitermarkus/verify

Refactor `Hbc::Verify`.

Library/Homebrew/cask/verify.rb

@@ -1,31 +1,24 @@
-require "cask/verify/checksum"
-
 module Hbc
   module Verify
     module_function
 
-    def verifications
-      [
-        Hbc::Verify::Checksum,
-      ]
-    end
-
     def all(cask, downloaded_path)
-      odebug "Verifying download"
+      if cask.sha256 == :no_check
-      verifications = for_cask(cask)
+        ohai "No SHA-256 checksum defined for Cask '#{cask}', skipping verification."
-      odebug "#{verifications.size} verifications defined", verifications
+        return
-      verifications.each do |verification|
-        odebug "Running verification of class #{verification}"
-        verification.new(cask, downloaded_path).verify
       end
-    end
 
-    def for_cask(cask)
+      ohai "Verifying SHA-256 checksum for Cask '#{cask}'."
-      odebug "Determining which verifications to run for Cask #{cask}"
+
-      verifications.select do |verification|
+      expected = cask.sha256
-        odebug "Checking for verification class #{verification}"
+      computed = downloaded_path.sha256
-        verification.me?(cask)
+
-      end
+      raise CaskSha256MissingError.new(cask.token, expected, computed) if expected.nil? || expected.empty?
+
+      return if expected == computed
+
+      ohai "Note: Running `brew update` may fix SHA-256 checksum errors."
+      raise CaskSha256MismatchError.new(cask.token, expected, computed, downloaded_path)
     end
   end
 end

Library/Homebrew/cask/verify/checksum.rb

@@ -1,47 +0,0 @@
-require "digest"
-
-module Hbc
-  module Verify
-    class Checksum
-      def self.me?(cask)
-        return true unless cask.sha256 == :no_check
-        ohai "No checksum defined for Cask #{cask}, skipping verification"
-        false
-      end
-
-      attr_reader :cask, :downloaded_path
-
-      def initialize(cask, downloaded_path)
-        @cask = cask
-        @downloaded_path = downloaded_path
-      end
-
-      def verify
-        return unless self.class.me?(cask)
-        ohai "Verifying SHA-256 checksum for Cask '#{cask}'."
-        verify_checksum
-      end
-
-      private
-
-      def expected
-        @expected ||= cask.sha256
-      end
-
-      def computed
-        @computed ||= downloaded_path.sha256
-      end
-
-      def verify_checksum
-        raise CaskSha256MissingError.new(cask.token, expected, computed) if expected.nil? || expected.empty?
-
-        if expected == computed
-          odebug "SHA-256 checksums match."
-        else
-          ohai 'Note: running "brew update" may fix sha256 checksum errors'
-          raise CaskSha256MismatchError.new(cask.token, expected, computed, downloaded_path)
-        end
-      end
-    end
-  end
-end

Library/Homebrew/test/cask/verify/checksum_spec.rb

@@ -1,88 +0,0 @@
-describe Hbc::Verify::Checksum, :cask do
-  let(:cask) { double("cask", token: "cask") }
-  let(:downloaded_path) { instance_double("Pathname") }
-  let(:verification) { described_class.new(cask, downloaded_path) }
-
-  before do
-    allow(cask).to receive(:sha256).and_return(expected_sha256)
-  end
-
-  describe ".me?" do
-    subject { described_class.me?(cask) }
-
-    context "when expected sha256 is :no_check" do
-      let(:expected_sha256) { :no_check }
-
-      it { is_expected.to be false }
-    end
-
-    context "when expected sha256 is nil" do
-      let(:expected_sha256) { nil }
-
-      it { is_expected.to be true }
-    end
-
-    context "sha256 is empty" do
-      let(:expected_sha256) { "" }
-
-      it { is_expected.to be true }
-    end
-
-    context "when expected sha256 is a valid shasum" do
-      let(:expected_sha256) { "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef" }
-
-      it { is_expected.to be true }
-    end
-  end
-
-  describe "#verify" do
-    subject { verification.verify }
-
-    before do
-      allow(cask).to receive(:sha256).and_return(expected_sha256)
-      allow(downloaded_path).to receive(:sha256).and_return(actual_sha256)
-    end
-
-    let(:actual_sha256) { "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef" }
-
-    context "when expected matches actual sha256" do
-      let(:expected_sha256) { actual_sha256 }
-
-      it "does not raise an error" do
-        expect { subject }.not_to raise_error
-      end
-    end
-
-    context "when expected sha256 is :no_check" do
-      let(:expected_sha256) { :no_check }
-
-      it "does not raise an error" do
-        expect { subject }.not_to raise_error
-      end
-    end
-
-    context "when expected does not match sha256" do
-      let(:expected_sha256) { "d3adb33fd3adb33fd3adb33fd3adb33fd3adb33fd3adb33fd3adb33fd3adb33f" }
-
-      it "raises an error" do
-        expect { subject }.to raise_error(Hbc::CaskSha256MismatchError)
-      end
-    end
-
-    context "when expected sha256 is nil" do
-      let(:expected_sha256) { nil }
-
-      it "raises an error" do
-        expect { subject }.to raise_error(Hbc::CaskSha256MissingError)
-      end
-    end
-
-    context "when expected sha256 is empty" do
-      let(:expected_sha256) { "" }
-
-      it "raises an error" do
-        expect { subject }.to raise_error(Hbc::CaskSha256MissingError)
-      end
-    end
-  end
-end

Library/Homebrew/test/cask/verify_spec.rb

@@ -1,63 +1,52 @@
-describe Hbc::Verify, :cask do
+module Hbc
-  let(:cask) { double("cask") }
+  describe Verify, :cask do
+    describe "::all" do
+      subject(:verification) { described_class.all(cask, downloaded_path) }
+      let(:cask) { instance_double(Cask, token: "cask", sha256: expected_sha256) }
+      let(:cafebabe) { "cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe" }
+      let(:deadbeef) { "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef" }
+      let(:computed_sha256) { cafebabe }
+      let(:downloaded_path) { instance_double(Pathname, sha256: computed_sha256) }
 
-  let(:verification_classes) {
+      context "when the expected checksum is :no_check" do
-    [
+        let(:expected_sha256) { :no_check }
-      applicable_verification_class,
-      inapplicable_verification_class,
-    ]
-  }
 
-  let(:applicable_verification_class) {
+        it "skips the check" do
-    double("applicable_verification_class", me?: true)
+          expect { verification }.to output(/skipping verification/).to_stdout
-  }
+        end
-
-  let(:inapplicable_verification_class) {
-    double("inapplicable_verification_class", me?: false)
-  }
-
-  before do
-    allow(described_class).to receive(:verifications)
-      .and_return(verification_classes)
-  end
-
-  describe ".for_cask" do
-    subject { described_class.for_cask(cask) }
-
-    it "checks applicability of each verification" do
-      verification_classes.each do |verify_class|
-        expect(verify_class).to receive(:me?).with(cask)
       end
-      subject
-    end
 
-    it "includes applicable verifications" do
+      context "when expected and computed checksums match" do
-      expect(subject).to include(applicable_verification_class)
+        let(:expected_sha256) { cafebabe }
-    end
 
-    it "excludes inapplicable verifications" do
+        it "does not raise an error" do
-      expect(subject).not_to include(inapplicable_verification_class)
+          expect { verification }.not_to raise_error
-    end
+        end
-  end
+      end
 
-  describe ".all" do
+      context "when the expected checksum is nil" do
-    subject { described_class.all(cask, downloaded_path) }
+        let(:expected_sha256) { nil }
 
-    let(:downloaded_path) { double("downloaded_path") }
+        it "raises an error" do
-    let(:applicable_verification) { double("applicable_verification") }
+          expect { verification }.to raise_error CaskSha256MissingError
-    let(:inapplicable_verification) { double("inapplicable_verification") }
+        end
+      end
 
-    before do
+      context "when the expected checksum is empty" do
-      allow(applicable_verification_class).to receive(:new)
+        let(:expected_sha256) { "" }
-        .and_return(applicable_verification)
-      allow(inapplicable_verification_class).to receive(:new)
-        .and_return(inapplicable_verification)
-    end
 
-    it "runs only applicable verifications" do
+        it "raises an error" do
-      expect(applicable_verification).to receive(:verify)
+          expect { verification }.to raise_error CaskSha256MissingError
-      expect(inapplicable_verification).not_to receive(:verify)
+        end
-      subject
+      end
+
+      context "when expected and computed checksums do not match" do
+        let(:expected_sha256) { deadbeef }
+
+        it "raises an error" do
+          expect { verification }.to raise_error CaskSha256MismatchError
+        end
+      end
     end
   end
 end