From 89fa6b88c2d31c04c024858065ac64e9c610aa76 Mon Sep 17 00:00:00 2001 From: "Daniel (dB.) Doubrovkine" Date: Fri, 4 Sep 2026 07:27:10 -0400 Subject: [PATCH] Improve branch coverage for exceptions, namespace, router, testing and translation utilities Adds direct unit tests to exercise previously-uncovered branches: - Grape::Exceptions::Base#translate_message (Proc/Hash-pattern/plain-value branches) - Grape::Exceptions::RequestError (new spec file) - Grape::Exceptions::Validation (message omitted case) - Grape::Middleware::Base (instance-level #default_options fallback) - Grape::Middleware::Error (#resolved_backtrace fallback chain, InvalidVersionHeader precedence) - Grape::Namespace.joined_space - Grape::Router::BaseRoute#initialize (ActiveSupport::OrderedOptions wrapping) - Grape::Router::Pattern::Path#suffix (non-path versioning) - Grape::Testing (missing block / no registered hooks) - Grape::Util::Translation#translate (explicit locale / explicit default) Raises the full-matrix branch coverage from 95.58% (1278/1337) to 96.40% (1289/1337), while keeping line coverage at 100%. Not aiming for 100% branch coverage; only the easy, high-signal gaps were closed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 1 + spec/grape/exceptions/base_spec.rb | 28 +++++++++++++++++ spec/grape/exceptions/request_error_spec.rb | 34 +++++++++++++++++++++ spec/grape/exceptions/validation_spec.rb | 12 ++++++++ spec/grape/middleware/base_spec.rb | 18 +++++++++++ spec/grape/middleware/error_spec.rb | 30 ++++++++++++++++++ spec/grape/namespace_spec.rb | 11 +++++++ spec/grape/router/base_route_spec.rb | 29 ++++++++++++++++++ spec/grape/router/pattern/path_spec.rb | 9 ++++++ spec/grape/testing_spec.rb | 10 ++++++ spec/grape/util/translation_spec.rb | 13 ++++++++ 11 files changed, 195 insertions(+) create mode 100644 spec/grape/exceptions/request_error_spec.rb create mode 100644 spec/grape/router/base_route_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 696b9e954..80fa675a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,7 @@ * [#2893](https://github.com/ruby-grape/grape/pull/2893): Scrub the format extension rather than the whole request path when negotiating a format - [@ericproulx](https://github.com/ericproulx). * [#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). * [#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 diff --git a/spec/grape/exceptions/base_spec.rb b/spec/grape/exceptions/base_spec.rb index aabadce02..5c718be54 100644 --- a/spec/grape/exceptions/base_spec.rb +++ b/spec/grape/exceptions/base_spec.rb @@ -136,4 +136,32 @@ 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 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 diff --git a/spec/grape/exceptions/request_error_spec.rb b/spec/grape/exceptions/request_error_spec.rb new file mode 100644 index 000000000..a2138b7b5 --- /dev/null +++ b/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 diff --git a/spec/grape/exceptions/validation_spec.rb b/spec/grape/exceptions/validation_spec.rb index ab147129b..fd4c5390b 100644 --- a/spec/grape/exceptions/validation_spec.rb +++ b/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) } diff --git a/spec/grape/middleware/base_spec.rb b/spec/grape/middleware/base_spec.rb index 55c441179..4467aec8c 100644 --- a/spec/grape/middleware/base_spec.rb +++ b/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 diff --git a/spec/grape/middleware/error_spec.rb b/spec/grape/middleware/error_spec.rb index 784e5d17f..db48f6f93 100644 --- a/spec/grape/middleware/error_spec.rb +++ b/spec/grape/middleware/error_spec.rb @@ -463,4 +463,34 @@ def initialize expect(middleware.__send__(:error?, 'not an error')).to be false end end + + describe '#resolved_backtrace' do + subject(:middleware) { described_class.new(->(_env) {}, rescue_options: Grape::DSL::RescueOptions.new(backtrace: true)) } + + context 'when the raw response has no backtrace of its own' do + it 'falls back to the original exception backtrace' do + original_exception = RuntimeError.new('boom') + original_exception.set_backtrace(['original.rb:1']) + raw = Grape::Exceptions::ErrorResponse.new(original_exception:) + + expect(middleware.__send__(:resolved_backtrace, raw)).to eq(['original.rb:1']) + end + end + + context 'when neither the raw response nor the original exception have a backtrace' do + it 'returns an empty array' do + raw = Grape::Exceptions::ErrorResponse.new + expect(middleware.__send__(:resolved_backtrace, raw)).to eq([]) + end + end + end + + describe '#grape_exceptions_precedence_handler' do + subject(:middleware) { described_class.new(->(_env) {}, rescue_grape_exceptions: true) } + + it 'leaves InvalidVersionHeader alone so it keeps reaching Rack' do + handler = middleware.__send__(:grape_exceptions_precedence_handler, Grape::Exceptions::InvalidVersionHeader, nil) + expect(handler).to be_nil + end + end end diff --git a/spec/grape/namespace_spec.rb b/spec/grape/namespace_spec.rb index bd2f9e907..ace83de7e 100644 --- a/spec/grape/namespace_spec.rb +++ b/spec/grape/namespace_spec.rb @@ -30,4 +30,15 @@ expect(namespace.hash).not_to eq(other.hash) end end + + describe '.joined_space' do + it 'maps a list of Namespace objects to their #space' do + other = described_class.new('bar') + expect(described_class.joined_space([namespace, other])).to eq(%w[foo bar]) + end + + it 'returns nil for a nil settings list' do + expect(described_class.joined_space(nil)).to be_nil + end + end end diff --git a/spec/grape/router/base_route_spec.rb b/spec/grape/router/base_route_spec.rb new file mode 100644 index 000000000..646bc48c6 --- /dev/null +++ b/spec/grape/router/base_route_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +describe Grape::Router::BaseRoute do + let(:pattern) { instance_double(Grape::Router::Pattern) } + + describe '#initialize' do + context 'when options is a plain Hash' do + subject(:route) { described_class.new(pattern, { foo: 'bar' }) } + + it 'wraps it in an ActiveSupport::OrderedOptions' do + expect(route.options).to be_a(ActiveSupport::OrderedOptions) + end + + it 'reads back the given options' do + expect(route.options[:foo]).to eq('bar') + end + end + + context 'when options is already an ActiveSupport::OrderedOptions' do + subject(:route) { described_class.new(pattern, options) } + + let(:options) { ActiveSupport::OrderedOptions.new.update(foo: 'bar') } + + it 'uses it as-is, without wrapping it again' do + expect(route.options).to equal(options) + end + end + end +end diff --git a/spec/grape/router/pattern/path_spec.rb b/spec/grape/router/pattern/path_spec.rb index 79fdf8267..7cc2b0047 100644 --- a/spec/grape/router/pattern/path_spec.rb +++ b/spec/grape/router/pattern/path_spec.rb @@ -73,6 +73,15 @@ def path_settings(**attrs) end end + context 'when versioning is used but not via path (e.g. header)' do + it "does not include a '/'" do + path = described_class.new( + nil, nil, path_settings(version: :v1, version_options: Grape::DSL::VersionOptions.new(using: :header)) + ) + expect(path.suffix).to eql('(.:format)') + end + end + context 'when path versioning is not used' do it "does not include a '/' when the path has a namespace" do path = described_class.new(nil, 'namespace', path_settings) diff --git a/spec/grape/testing_spec.rb b/spec/grape/testing_spec.rb index 6e9725934..bced11dd8 100644 --- a/spec/grape/testing_spec.rb +++ b/spec/grape/testing_spec.rb @@ -42,5 +42,15 @@ Grape::Endpoint.reset_before_each expect { get '/' }.to raise_error(NoMethodError, /undefined method [`']authenticate_user!' for/) end + + it 'raises an ArgumentError when no block is given' do + expect { Grape::Endpoint.before_each }.to raise_error(ArgumentError, 'a block is required') + end + + it 'does nothing when no before_each hooks were registered' do + subject.get('/') { 'hello' } + expect { get '/' }.not_to raise_error + expect(last_response.body).to eq('hello') + end end end diff --git a/spec/grape/util/translation_spec.rb b/spec/grape/util/translation_spec.rb index bded528e0..effae1871 100644 --- a/spec/grape/util/translation_spec.rb +++ b/spec/grape/util/translation_spec.rb @@ -24,5 +24,18 @@ def translate_message(key, **opts) expect { translator.translate_message(:reserved_key_test) }.to raise_error(I18n::ReservedInterpolationKey) end end + + context 'when an explicit locale is given' do + it 'passes the locale through to I18n.translate' do + expect(I18n).to receive(:translate).with(:missing_key, hash_including(locale: :en)).and_call_original + translator.translate_message(:missing_key, locale: :en) + end + end + + context 'when an explicit default is given and the key is missing' do + it 'returns the given default instead of the dotted key path' do + expect(translator.translate_message(:missing_key, default: 'fallback')).to eq('fallback') + end + end end end