Improve branch test coverage

CHANGELOG.md

@@ -69,6 +69,8 @@
 * [#2894](https://github.com/ruby-grape/grape/pull/2894): Read the request method once in `default_status` instead of asking through `post?` and `delete?` - [@ericproulx](https://github.com/ericproulx).
 * [#2895](https://github.com/ruby-grape/grape/pull/2895): Cut per-request work out of the endpoint, validation and router paths: scan the router's compiled union by capture number rather than by name, skip the Array boxing in `AttributesIterator` for a flat scope, read a coerced attribute once instead of three times, and resolve the formatter's config and the coercers' type checks once at build time - [@ericproulx](https://github.com/ericproulx).
 * [#2900](https://github.com/ruby-grape/grape/pull/2900): Remove the deprecations announced in 3.2 and 3.3: `Grape::Router.normalize_path`, Hash access on middleware `Options` and their `DEFAULT_OPTIONS` constants, the positional options Hash for `auth`/`http_basic`/`desc`, a Hash returned from a `rescue_from` handler, and `@option` on validators (see UPGRADING) - [@ericproulx](https://github.com/ericproulx).
+* [#2896](https://github.com/ruby-grape/grape/pull/2896): Bring test suite line coverage to 100% - [@dblock](https://github.com/dblock).
+* [#2897](https://github.com/ruby-grape/grape/pull/2897): Improve test suite branch coverage - [@dblock](https://github.com/dblock).
 * Your contribution here.
 
 #### Fixes

spec/grape/api/instance_spec.rb

@@ -105,4 +105,16 @@ def app
       expect(last_response.body).to eq 'Not found! (2)'
     end
   end
+
+  describe '.cascade?' do
+    subject(:an_instance) do
+      Class.new(Grape::API::Instance) do
+        cascade true
+      end
+    end
+
+    it 'returns the configured cascade setting' do
+      expect(an_instance.compile!.cascade?).to be(true)
+    end
+  end
 end

spec/grape/api_spec.rb

@@ -2090,6 +2090,17 @@ def hello
       expect(last_response.body).to eql 'Hello, world.'
     end
 
+    it 'includes all known helpers in scope when called with no args and no block' do
+      subject.helpers do
+        def hello
+          'Hello, world.'
+        end
+      end
+
+      new_mod = subject.helpers
+      expect(new_mod.instance_methods).to include(:hello)
+    end
+
     it 'is scopable' do
       subject.helpers do
         def generic

spec/grape/dsl/request_response_spec.rb

@@ -134,6 +134,12 @@
         expect(subject.inheritable_setting.all_rescue_handler).to eq(with_block)
       end
 
+      it 'converts a String :with option to a Symbol' do
+        subject.rescue_from :all, with: 'my_handler'
+        expect(subject.inheritable_setting.rescue_all?).to be(true)
+        expect(subject.inheritable_setting.all_rescue_handler).to eq(:my_handler)
+      end
+
       it 'abort if :with option value is not Symbol, String or Proc' do
         expect { subject.rescue_from :all, with: 1234 }.to raise_error(ArgumentError, "with: #{integer_class_name}, expected Symbol, String or Proc")
       end

spec/grape/endpoint_spec.rb

@@ -1042,6 +1042,9 @@ def memoized
       subject.before do
         # Placeholder
       end
+      subject.params do
+        optional :id, type: Integer
+      end
       subject.get do
         'hello'
       end
@@ -1064,7 +1067,10 @@ def memoized
         have_attributes(name: 'endpoint_run_filters.grape', payload: { endpoint: a_kind_of(described_class),
                                                                        filters: a_collection_containing_exactly(an_instance_of(Proc)),
                                                                        type: :before }),
-        have_attributes(name: 'endpoint_render.grape',      payload: { endpoint: a_kind_of(described_class) }),
+        have_attributes(name: 'endpoint_run_validators.grape', payload: { endpoint: a_kind_of(described_class),
+                                                                          validators: an_instance_of(Array),
+                                                                          request: a_kind_of(Grape::Request) }),
+        have_attributes(name: 'endpoint_render.grape', payload: { endpoint: a_kind_of(described_class) }),
         have_attributes(name: 'endpoint_run.grape', payload: { endpoint: a_kind_of(described_class),
                                                                env: an_instance_of(Hash) }),
         have_attributes(name: 'format_response.grape', payload: { env: an_instance_of(Hash),
@@ -1078,7 +1084,10 @@ def memoized
         have_attributes(name: 'endpoint_run_filters.grape', payload: { endpoint: a_kind_of(described_class),
                                                                        filters: a_collection_containing_exactly(an_instance_of(Proc)),
                                                                        type: :before }),
-        have_attributes(name: 'endpoint_render.grape',      payload: { endpoint: a_kind_of(described_class) }),
+        have_attributes(name: 'endpoint_run_validators.grape', payload: { endpoint: a_kind_of(described_class),
+                                                                          validators: an_instance_of(Array),
+                                                                          request: a_kind_of(Grape::Request) }),
+        have_attributes(name: 'endpoint_render.grape', payload: { endpoint: a_kind_of(described_class) }),
         have_attributes(name: 'format_response.grape', payload: { env: an_instance_of(Hash),
                                                                   formatter: a_kind_of(Module) })
       )
@@ -1126,5 +1135,20 @@ def memoized
     it 'does not raise an error' do
       expect { subject }.not_to raise_error
     end
+
+    context 'when the endpoint has handled a request (env is set)' do
+      it 'includes the route origin in the inspect output' do
+        inspect_output = nil
+        api = Class.new(Grape::API) do
+          get('/hello') do
+            inspect_output = inspect
+            'world'
+          end
+        end
+        env = Rack::MockRequest.env_for('/hello')
+        api.call(env)
+        expect(inspect_output).to eq("#{described_class} in '/hello' endpoint")
+      end
+    end
   end
 end

spec/grape/error_formatter/base_spec.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+describe Grape::ErrorFormatter::Base do
+  describe '.format_structured_message' do
+    it 'raises NotImplementedError' do
+      expect { described_class.format_structured_message({}) }.to raise_error(NotImplementedError)
+    end
+  end
+end

spec/grape/exceptions/base_spec.rb

@@ -136,4 +136,40 @@
       end
     end
   end
+
+  describe '#translate_message (private)' do
+    subject(:translate_message) { described_class.new.__send__(:translate_message, translation_key) }
+
+    context 'when given a Proc' do
+      let(:translation_key) { -> { 'from a proc' } }
+
+      it 'calls the Proc' do
+        expect(translate_message).to eq('from a proc')
+      end
+    end
+
+    context 'when given a Hash matching {key:, **opts}' do
+      let(:translation_key) { { key: :invalid_formatter, klass: String, to_format: 'xml' } }
+
+      it 'translates using the key and forwards the remaining pairs as opts' do
+        expect(translate_message).to eq('cannot convert String to xml')
+      end
+    end
+
+    context 'when given a Hash that does not have a :key entry' do
+      let(:translation_key) { { klass: String, to_format: 'xml' } }
+
+      it 'raises NoMatchingPatternKeyError' do
+        expect { translate_message }.to raise_error(NoMatchingPatternKeyError)
+      end
+    end
+
+    context 'when given anything else (e.g. a plain String)' do
+      let(:translation_key) { 'a literal message' }
+
+      it 'returns it unchanged' do
+        expect(translate_message).to eq('a literal message')
+      end
+    end
+  end
 end

spec/grape/exceptions/request_error_spec.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+describe Grape::Exceptions::RequestError do
+  describe '#initialize' do
+    context 'when raised inside a rescue block' do
+      it 'captures the current exception message' do
+        error = begin
+          raise 'boom'
+        rescue RuntimeError
+          described_class.new
+        end
+        expect(error.message).to eq('boom')
+      end
+    end
+
+    context 'when there is no current exception' do
+      it 'has no message from a prior exception' do
+        # $ERROR_INFO ($!) is read-only and only set by an active rescue, so
+        # simulate "no exception" the same way: outside any rescue block.
+        # `StandardError#message` defaults to the class name when no message
+        # was given, so this pins the $ERROR_INFO&.message safe-nav's nil case.
+        expect(described_class.new.message).to eq(described_class.name)
+      end
+    end
+
+    it 'defaults status to 400' do
+      expect(described_class.new.status).to eq(400)
+    end
+
+    it 'accepts a custom status' do
+      expect(described_class.new(status: 422).status).to eq(422)
+    end
+  end
+end

spec/grape/exceptions/validation_errors_spec.rb

@@ -37,6 +37,13 @@
     end
   end
 
+  describe '#to_json' do
+    it 'returns the JSON representation of #as_json' do
+      error = described_class.new(exceptions: [validation_error])
+      expect(error.to_json).to eq(error.as_json.to_json)
+    end
+  end
+
   describe '#full_messages' do
     context 'with errors' do
       subject { described_class.new(exceptions: [validation_error_1, validation_error_2]).full_messages }

spec/grape/exceptions/validation_spec.rb

@@ -5,6 +5,18 @@
     expect { described_class.new(message: 'presence') }.to raise_error(ArgumentError, /missing keyword:.+?params/)
   end
 
+  context 'when message is omitted' do
+    subject(:error) { described_class.new(params: ['id']) }
+
+    it 'has a nil message_key' do
+      expect(error.message_key).to be_nil
+    end
+
+    it 'has no message from the given options' do
+      expect(error.message).to eq(described_class.name)
+    end
+  end
+
   context 'when message is a Symbol' do
     subject(:error) { described_class.new(params: ['id'], message: :presence) }
 
@@ -52,4 +64,11 @@
       expect(error.message).to eq('raw message')
     end
   end
+
+  describe '#as_json' do
+    it 'returns the string representation of the error' do
+      error = described_class.new(params: ['id'], message: 'raw message')
+      expect(error.as_json).to eq(error.to_s)
+    end
+  end
 end

spec/grape/formatter/base_spec.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+describe Grape::Formatter::Base do
+  describe '.call' do
+    it 'raises NotImplementedError' do
+      expect { described_class.call({}, {}) }.to raise_error(NotImplementedError)
+    end
+  end
+end

spec/grape/formatter/json_spec.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+describe Grape::Formatter::Json do
+  describe '.call' do
+    it 'returns the string representation of a Grape::PrecompiledJson object' do
+      precompiled = Grape::PrecompiledJson.new('{"a":1}')
+      expect(described_class.call(precompiled, {})).to eq('{"a":1}')
+    end
+
+    it 'calls #to_json when the object responds to it' do
+      object = { a: 1 }
+      expect(described_class.call(object, {})).to eq(object.to_json)
+    end
+
+    it 'falls back to Grape::Json.dump when the object does not respond to #to_json' do
+      object = Object.new
+      allow(object).to receive(:respond_to?).with(:to_json).and_return(false)
+      expect(described_class.call(object, {})).to eq(Grape::Json.dump(object))
+    end
+  end
+end

spec/grape/formatter/serializable_hash_spec.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+describe Grape::Formatter::SerializableHash do
+  describe '.call' do
+    it 'returns the string representation of a Grape::PrecompiledJson object' do
+      precompiled = Grape::PrecompiledJson.new('{"a":1}')
+      expect(described_class.call(precompiled, {})).to eq('{"a":1}')
+    end
+
+    it 'returns a String object unchanged' do
+      expect(described_class.call('already a string', {})).to eq('already a string')
+    end
+
+    it 'serializes an object responding to #serializable_hash' do
+      object = Class.new do
+        def serializable_hash
+          { a: 1 }
+        end
+      end.new
+      expect(described_class.call(object, {})).to eq(Grape::Json.dump(a: 1))
+    end
+
+    it 'serializes an Array of objects responding to #serializable_hash' do
+      klass = Class.new do
+        def initialize(value)
+          @value = value
+        end
+
+        def serializable_hash
+          { value: @value }
+        end
+      end
+      objects = [klass.new(1), klass.new(2)]
+      expect(described_class.call(objects, {})).to eq(Grape::Json.dump([{ value: 1 }, { value: 2 }]))
+    end
+
+    it 'serializes a Hash, recursively serializing its values' do
+      object = Class.new do
+        def serializable_hash
+          { a: 1 }
+        end
+      end.new
+      expect(described_class.call({ nested: object }, {})).to eq(Grape::Json.dump(nested: { a: 1 }))
+    end
+
+    it 'serializes a Hash, leaving non-serializable leaf values untouched' do
+      expect(described_class.call({ plain: 'value' }, {})).to eq(Grape::Json.dump(plain: 'value'))
+    end
+
+    it 'calls #to_json when the object responds to it and is not otherwise serializable' do
+      object = 1234
+      expect(described_class.call(object, {})).to eq(object.to_json)
+    end
+
+    it 'falls back to Grape::Json.dump when the object is not serializable and does not respond to #to_json' do
+      object = Object.new
+      allow(object).to receive(:respond_to?).and_call_original
+      allow(object).to receive(:respond_to?).with(:to_json).and_return(false)
+      expect(described_class.call(object, {})).to eq(Grape::Json.dump(object))
+    end
+  end
+end

spec/grape/middleware/base_spec.rb

@@ -189,6 +189,24 @@
       end
     end
 
+    context 'when a middleware defines an instance-level #default_options' do
+      let(:example_ware) do
+        Class.new(Grape::Middleware::Base) do
+          def default_options
+            { monkey: true }
+          end
+        end
+      end
+
+      it 'merges options through the instance method instead of a constant' do
+        expect(example_ware.new(blank_app).options[:monkey]).to be true
+      end
+
+      it 'overrides default options when provided' do
+        expect(example_ware.new(blank_app, monkey: false).options[:monkey]).to be false
+      end
+    end
+
     context 'when a middleware declares its own Options Data class' do
       let(:example_ware) do
         Class.new(Grape::Middleware::Base) do
@@ -251,6 +269,23 @@ def after
       expect(last_response.headers['X-Test-Before']).to eq('Hi')
       expect(last_response.headers['X-Test-After']).to eq('Bye')
     end
+
+    context 'when the downstream app returns a Rack::Response' do
+      let(:app) do
+        context = self
+
+        Rack::Builder.app do
+          use context.example_ware
+          run ->(_) { Rack::Response.new('Yeah', 200, {}) }
+        end
+      end
+
+      it 'merges the header onto the Rack::Response' do
+        get '/'
+        expect(last_response.headers['X-Test-Before']).to eq('Hi')
+        expect(last_response.headers['X-Test-After']).to eq('Bye')
+      end
+    end
   end
 
   context 'header overwrite' do