Merge pull request #3 from blocknotes/feature/spec-improvements

Feature: spec improvements

Author: blocknotes

.rubocop.yml

@@ -1,6 +1,8 @@
 inherit_from:
   - https://relaxed.ruby.style/rubocop.yml
 
+require: rubocop-rspec
+
 AllCops:
   TargetRubyVersion: 2.6
   Exclude:
@@ -12,6 +14,17 @@ AllCops:
 Gemspec/RequiredRubyVersion:
   Enabled: false
 
-Layout/LineLength:
+Metrics/LineLength:
   Enabled: true
   Max: 120
+
+RSpec/MultipleExpectations:
+  # Default is 1
+  Max: 5
+
+RSpec/NestedGroups:
+  # Default is 3
+  Max: 5
+
+RSpec/ExampleLength:
+  Max: 6

active_storage_db.gemspec

@@ -39,6 +39,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'rspec_junit_formatter', '~> 0.4.1'
   spec.add_development_dependency 'rspec-rails', '~> 4.0.1'
   spec.add_development_dependency 'rubocop', '~> 0.89.0'
+  spec.add_development_dependency 'rubocop-rspec', '~> 1.42.0'
   spec.add_development_dependency 'selenium-webdriver', '~> 3.142.7'
   spec.add_development_dependency 'simplecov', '~> 0.18.5'
 end

spec/factories/active_storage_db_files.rb

@@ -4,6 +4,8 @@
   factory :active_storage_db_file, class: 'ActiveStorageDB::File' do
     ref { ('a'..'z').to_a.shuffle[0, 32].join } # rubocop:disable Style/Sample
 
-    data { "<?xml version=\"1.0\" encoding=\"iso-8859-1\"?>\n<svg version=\"1.1\" id=\"Capa_1\" xmlns=\"http://www.w3.org/2000/svg\" xmlns:xlink=\"http://www.w3.org/1999/xlink\" x=\"0px\" y=\"0px\" viewBox=\"0 0 299.429 299.429\" style=\"enable-background:new 0 0 299.429 299.429;\" xml:space=\"preserve\">\n<g>\n<path style=\"fill:#010002;\" d=\"M245.185,44.209H54.245L0,116.533l149.715,138.688l149.715-138.682L245.185,44.209z\nM206.746,121.778l-57.007,112.1l-56.53-112.1H206.746z M98.483,109.844l51.232-51.232l51.232,51.232H98.483z M164.119,56.142\nh69.323L213.876,105.9L164.119,56.142z M86.311,105.142l-16.331-49h65.331L86.311,105.142z M79.849,121.778l49.632,98.429\nL23.223,121.778H79.849z M220.136,121.778h56.071l-106.013,98.203L220.136,121.778z M225.148,109.844l18.694-47.538l35.652,47.538\nH225.148z M58.266,58.738l17.035,51.112H19.929L58.266,58.738z\"\n/>\n</g>\n</svg>\n" } # rubocop:disable Layout/LineLength
+    data do
+      (+"\211PNG\r\n\032\n\000\000\000\rIHDR\000\000\000\020\000\000\000\020\001\003\000\000\000%=m\"\000\000\000\006PLTE\000\000\000\377\377\377\245\331\237\335\000\000\0003IDATx\234c\370\377\237\341\377_\206\377\237\031\016\2603\334?\314p\1772\303\315\315\f7\215\031\356\024\203\320\275\317\f\367\201R\314\f\017\300\350\377\177\000Q\206\027(\316]\233P\000\000\000\000IEND\256B`\202").force_encoding(Encoding::BINARY) # rubocop:disable Layout/LineLength
+    end
   end
 end

spec/models/file_spec.rb …ec/models/active_storage_db/file_spec.rbspec/models/file_spec.rb renamed to spec/models/active_storage_db/file_spec.rb spec/models/file_spec.rb renamed to spec/models/active_storage_db/file_spec.rb

@@ -9,8 +9,9 @@ def create_blob_before_direct_upload(filename: 'hello.txt', byte_size:, checksum
     ActiveStorage::Blob.create_before_direct_upload! filename: filename, byte_size: byte_size, checksum: checksum, content_type: content_type # rubocop:disable Layout/LineLength
   end
 
-  let(:blob) { create_blob(filename: 'hello.jpg', content_type: 'image/jpg') }
+  let(:blob) { create_blob(filename: 'img.jpg', content_type: 'image/jpg') }
   let(:host) { 'http://test.example.com:3001' }
+  let(:engine_url_helpers) { ::ActiveStorageDB::Engine.routes.url_helpers }
 
   before { allow(ActiveStorage::Current).to receive(:host).and_return(host) }
 
@@ -20,9 +21,8 @@ def create_blob_before_direct_upload(filename: 'hello.txt', byte_size:, checksum
 
       expect(response).to have_http_status(:ok)
       expect(response.content_type).to eq('image/jpg')
-      expect(response.headers['Content-Disposition']).to eq(
-        "inline; filename=\"hello.jpg\"; filename*=UTF-8''hello.jpg"
-      )
+      content_disposition = response.headers['Content-Disposition']
+      expect(content_disposition).to eq("inline; filename=\"img.jpg\"; filename*=UTF-8''img.jpg")
       expect(response.body).to eq 'Hello world!'
     end
 
@@ -31,9 +31,8 @@ def create_blob_before_direct_upload(filename: 'hello.txt', byte_size:, checksum
 
       expect(response).to have_http_status(:ok)
       expect(response.content_type).to eq('image/jpg')
-      expect(response.headers['Content-Disposition']).to eq(
-        "attachment; filename=\"hello.jpg\"; filename*=UTF-8''hello.jpg"
-      )
+      content_disposition = response.headers['Content-Disposition']
+      expect(content_disposition).to eq("attachment; filename=\"img.jpg\"; filename*=UTF-8''img.jpg")
       expect(response.body).to eq 'Hello world!'
     end
 
@@ -50,10 +49,9 @@ def create_blob_before_direct_upload(filename: 'hello.txt', byte_size:, checksum
     end
 
     context 'with an invalid key' do
-      let(:engine_url_helpers) { ::ActiveStorageDB::Engine.routes.url_helpers }
-
       it 'returns not found' do
         get engine_url_helpers.service_path(encoded_key: 'Invalid key', filename: 'hello.txt')
+
         expect(response).to have_http_status(:not_found)
       end
     end
@@ -74,7 +72,7 @@ def create_blob_before_direct_upload(filename: 'hello.txt', byte_size:, checksum
       put blob.service_url_for_direct_upload, params: data, headers: { 'Content-Type' => 'application/octet-stream' }
 
       expect(response).to have_http_status(:unprocessable_entity)
-      expect(blob.service.exist?(blob.key)).to be_falsey
+      expect(blob.service).not_to exist(blob.key)
     end
 
     context 'with an invalid checksum' do
@@ -86,7 +84,7 @@ def create_blob_before_direct_upload(filename: 'hello.txt', byte_size:, checksum
         put blob.service_url_for_direct_upload, params: data
 
         expect(response).to have_http_status(:unprocessable_entity)
-        expect(blob.service.exist?(blob.key)).to be_falsey
+        expect(blob.service).not_to exist(blob.key)
       end
     end
 
@@ -97,15 +95,14 @@ def create_blob_before_direct_upload(filename: 'hello.txt', byte_size:, checksum
         put blob.service_url_for_direct_upload, params: data, headers: { 'Content-Type' => 'text/plain' }
 
         expect(response).to have_http_status(:unprocessable_entity)
-        expect(blob.service.exist?(blob.key)).to be_falsey
+        expect(blob.service).not_to exist(blob.key)
       end
     end
 
     context 'with an invalid token' do
-      let(:engine_url_helpers) { ::ActiveStorageDB::Engine.routes.url_helpers }
-
       it 'returns not found' do
         put engine_url_helpers.update_service_path(encoded_token: 'Invalid token')
+
         expect(response).to have_http_status(:not_found)
       end
     end

spec/requests/file_controller_spec.rb

@@ -1,14 +1,12 @@
 # frozen_string_literal: true
 
 RSpec.describe ActiveStorage::Service::DBService do
-  let(:fixture_data) {
-    (+"\211PNG\r\n\032\n\000\000\000\rIHDR\000\000\000\020\000\000\000\020\001\003\000\000\000%=m\"\000\000\000\006PLTE\000\000\000\377\377\377\245\331\237\335\000\000\0003IDATx\234c\370\377\237\341\377_\206\377\237\031\016\2603\334?\314p\1772\303\315\315\f7\215\031\356\024\203\320\275\317\f\367\201R\314\f\017\300\350\377\177\000Q\206\027(\316]\233P\000\000\000\000IEND\256B`\202").force_encoding(Encoding::BINARY) # rubocop:disable Layout/LineLength
-  }
+  let(:fixture_data) { build(:active_storage_db_file).data }
   let(:content_type) { 'image/png' }
+  let(:host) { 'http://test.example.com:3001' }
   let(:checksum) { Digest::MD5.base64digest(fixture_data) }
   let(:key) { SecureRandom.base58(24) }
-  let(:tmp_config) { { tmp: { service: 'DB' } } }
-  let(:service) { described_class.configure(:tmp, tmp_config) }
+  let(:service) { described_class.configure(:tmp, tmp: { service: 'DB' }) }
   let(:upload_options) { {} }
   let(:upload) { service.upload(key, StringIO.new(fixture_data), upload_options) }
 
@@ -18,20 +16,22 @@
   end
 
   describe '.delete' do
+    subject(:delete) { service.delete(key) }
+
     before { upload }
 
     it 'deletes the file' do
-      expect(service.delete(key)).to be_kind_of ActiveStorageDB::File
-      expect(service.delete(key)).to be_nil
+      expect { delete }.to change { ActiveStorageDB::File.count }.from(1).to(0)
     end
   end
 
   describe '.delete_prefixed' do
+    subject(:delete_prefixed) { service.delete_prefixed(key[0..10]) }
+
+    before { upload }
+
     it 'deletes the files' do
-      file = upload
-      prefix = key[0..10]
-      expect(service.delete_prefixed(prefix)).to eq [file]
-      expect(service.delete_prefixed(prefix)).to be_empty
+      expect { delete_prefixed }.to change { ActiveStorageDB::File.count }.from(1).to(0)
     end
   end
 
@@ -48,11 +48,11 @@
       after { service.delete(key) }
 
       it 'downloads the data' do
-        is_expected.to eq fixture_data
+        expect(download).to eq fixture_data
       end
 
-      context 'with a block' do
-        it 'sends the data to the block' do
+      context 'with download a block' do
+        let(:download_block) do
           result = nil
           service.download(key) do |data|
             if result
@@ -61,31 +61,34 @@
               result = data
             end
           end
-          expect(result).to eq fixture_data
+          result
+        end
+
+        it 'sends the data to the block' do
+          expect(download_block).to eq fixture_data
         end
       end
     end
   end
 
   describe '.download_chunk' do
-    let(:subject) { service.download_chunk(key, range) }
-    let(:range) { (10..25) }
+    subject { service.download_chunk(key, range) }
+
+    let(:range) { (10..15) }
 
     before { upload }
 
     after { service.delete(key) }
 
-    it 'download a specific chunk of data' do
-      is_expected.to eq fixture_data[range]
-    end
+    it { is_expected.to eq fixture_data[range] }
   end
 
   describe '.exist?' do
-    subject(:exist) { service.exist?(key) }
+    subject { service.exist?(key) }
 
     it { is_expected.to be_falsey }
 
-    context 'after an upload' do
+    context 'when a file is uploaded' do
       before { upload }
 
       after { service.delete(key) }
@@ -95,11 +98,9 @@
   end
 
   describe '.headers_for_direct_upload' do
-    subject(:headers) { service.headers_for_direct_upload(key, content_type: content_type) }
+    subject { service.headers_for_direct_upload(key, content_type: content_type) }
 
-    it 'returns the headers' do
-      is_expected.to eq({ 'Content-Type' => content_type })
-    end
+    it { is_expected.to eq('Content-Type' => content_type) }
   end
 
   describe '.upload' do
@@ -129,11 +130,11 @@
   end
 
   describe '.url' do
-    let(:subject) do
-      filename = ActiveStorage::Filename.new('avatar.png')
+    subject do
       service.url(key, expires_in: 5.minutes, disposition: :inline, filename: filename, content_type: content_type)
     end
-    let(:host) { 'http://test.example.com:3001' }
+
+    let(:filename) { ActiveStorage::Filename.new('avatar.png') }
 
     before do
       upload
@@ -142,17 +143,15 @@
 
     after { service.delete(key) }
 
-    it 'generates the file URL' do
-      is_expected.to start_with host
-    end
+    it { is_expected.to start_with host }
   end
 
   describe '.url_for_direct_upload' do
+    subject { service.url_for_direct_upload(key, url_options) }
+
     let(:url_options) do
       { expires_in: 5.minutes, content_type: content_type, content_length: fixture_data.size, checksum: checksum }
     end
-    let(:subject) { service.url_for_direct_upload(key, url_options) }
-    let(:host) { 'http://test.example.com:3001' }
 
     before do
       upload
@@ -161,8 +160,6 @@
 
     after { service.delete(key) }
 
-    it 'generates the file URL' do
-      is_expected.to start_with host
-    end
+    it { is_expected.to start_with host }
   end
 end