Skip to content

Commit 7d49331

Browse files
dblockCopilot
andcommitted
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, and the Hash-without-:key error case) - 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::Middleware::Stack::Middleware#== (non-Middleware/non-Class comparison) - Grape::Middleware::Versioner::Header (skips media-type matching without a vendor) - Grape::Namespace.joined_space - Grape::Router::BaseRoute#initialize (ActiveSupport::OrderedOptions wrapping) - Grape::Router::MustermannPattern (:param capture syntax, Integer vs. default constraint) - Grape::Router::Pattern::Path#suffix (non-path versioning) - Grape::Testing (missing block / no registered hooks) - Grape::Util::Translation#translate (explicit locale / explicit default / fallback-locale retry) Also marks one genuinely unreachable branch (comment-only, not deleted, per project convention): - Grape::DSL::Routing#version's trailing `@versions&.last` — @versions is unconditionally reassigned to an Array a few lines above, so the `&.` can never see nil. Raises the full-matrix branch coverage from 95.58% (1278/1337) to 96.85% (1324/1367), 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>
1 parent 4c503ca commit 7d49331

14 files changed

Lines changed: 265 additions & 1 deletion

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
* [#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).
7171
* [#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).
7272
* [#2896](https://github.com/ruby-grape/grape/pull/2896): Bring test suite line coverage to 100% - [@dblock](https://github.com/dblock).
73+
* [#2897](https://github.com/ruby-grape/grape/pull/2897): Improve test suite branch coverage - [@dblock](https://github.com/dblock).
7374
* Your contribution here.
7475

7576
#### Fixes

spec/grape/exceptions/base_spec.rb

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,4 +136,40 @@
136136
end
137137
end
138138
end
139+
140+
describe '#translate_message (private)' do
141+
subject(:translate_message) { described_class.new.__send__(:translate_message, translation_key) }
142+
143+
context 'when given a Proc' do
144+
let(:translation_key) { -> { 'from a proc' } }
145+
146+
it 'calls the Proc' do
147+
expect(translate_message).to eq('from a proc')
148+
end
149+
end
150+
151+
context 'when given a Hash matching {key:, **opts}' do
152+
let(:translation_key) { { key: :invalid_formatter, klass: String, to_format: 'xml' } }
153+
154+
it 'translates using the key and forwards the remaining pairs as opts' do
155+
expect(translate_message).to eq('cannot convert String to xml')
156+
end
157+
end
158+
159+
context 'when given a Hash that does not have a :key entry' do
160+
let(:translation_key) { { klass: String, to_format: 'xml' } }
161+
162+
it 'raises NoMatchingPatternKeyError' do
163+
expect { translate_message }.to raise_error(NoMatchingPatternKeyError)
164+
end
165+
end
166+
167+
context 'when given anything else (e.g. a plain String)' do
168+
let(:translation_key) { 'a literal message' }
169+
170+
it 'returns it unchanged' do
171+
expect(translate_message).to eq('a literal message')
172+
end
173+
end
174+
end
139175
end
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# frozen_string_literal: true
2+
3+
describe Grape::Exceptions::RequestError do
4+
describe '#initialize' do
5+
context 'when raised inside a rescue block' do
6+
it 'captures the current exception message' do
7+
error = begin
8+
raise 'boom'
9+
rescue RuntimeError
10+
described_class.new
11+
end
12+
expect(error.message).to eq('boom')
13+
end
14+
end
15+
16+
context 'when there is no current exception' do
17+
it 'has no message from a prior exception' do
18+
# $ERROR_INFO ($!) is read-only and only set by an active rescue, so
19+
# simulate "no exception" the same way: outside any rescue block.
20+
# `StandardError#message` defaults to the class name when no message
21+
# was given, so this pins the $ERROR_INFO&.message safe-nav's nil case.
22+
expect(described_class.new.message).to eq(described_class.name)
23+
end
24+
end
25+
26+
it 'defaults status to 400' do
27+
expect(described_class.new.status).to eq(400)
28+
end
29+
30+
it 'accepts a custom status' do
31+
expect(described_class.new(status: 422).status).to eq(422)
32+
end
33+
end
34+
end

spec/grape/exceptions/validation_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,18 @@
55
expect { described_class.new(message: 'presence') }.to raise_error(ArgumentError, /missing keyword:.+?params/)
66
end
77

8+
context 'when message is omitted' do
9+
subject(:error) { described_class.new(params: ['id']) }
10+
11+
it 'has a nil message_key' do
12+
expect(error.message_key).to be_nil
13+
end
14+
15+
it 'has no message from the given options' do
16+
expect(error.message).to eq(described_class.name)
17+
end
18+
end
19+
820
context 'when message is a Symbol' do
921
subject(:error) { described_class.new(params: ['id'], message: :presence) }
1022

spec/grape/middleware/base_spec.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,24 @@
189189
end
190190
end
191191

192+
context 'when a middleware defines an instance-level #default_options' do
193+
let(:example_ware) do
194+
Class.new(Grape::Middleware::Base) do
195+
def default_options
196+
{ monkey: true }
197+
end
198+
end
199+
end
200+
201+
it 'merges options through the instance method instead of a constant' do
202+
expect(example_ware.new(blank_app).options[:monkey]).to be true
203+
end
204+
205+
it 'overrides default options when provided' do
206+
expect(example_ware.new(blank_app, monkey: false).options[:monkey]).to be false
207+
end
208+
end
209+
192210
context 'when a middleware declares its own Options Data class' do
193211
let(:example_ware) do
194212
Class.new(Grape::Middleware::Base) do

spec/grape/middleware/error_spec.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,4 +488,34 @@ def initialize
488488
expect(middleware.__send__(:error?, 'not an error')).to be false
489489
end
490490
end
491+
492+
describe '#resolved_backtrace' do
493+
subject(:middleware) { described_class.new(->(_env) {}, rescue_options: Grape::DSL::RescueOptions.new(backtrace: true)) }
494+
495+
context 'when the raw response has no backtrace of its own' do
496+
it 'falls back to the original exception backtrace' do
497+
original_exception = RuntimeError.new('boom')
498+
original_exception.set_backtrace(['original.rb:1'])
499+
raw = Grape::Exceptions::ErrorResponse.new(original_exception:)
500+
501+
expect(middleware.__send__(:resolved_backtrace, raw)).to eq(['original.rb:1'])
502+
end
503+
end
504+
505+
context 'when neither the raw response nor the original exception have a backtrace' do
506+
it 'returns an empty array' do
507+
raw = Grape::Exceptions::ErrorResponse.new
508+
expect(middleware.__send__(:resolved_backtrace, raw)).to eq([])
509+
end
510+
end
511+
end
512+
513+
describe '#grape_exceptions_precedence_handler' do
514+
subject(:middleware) { described_class.new(->(_env) {}, rescue_grape_exceptions: true) }
515+
516+
it 'leaves InvalidVersionHeader alone so it keeps reaching Rack' do
517+
handler = middleware.__send__(:grape_exceptions_precedence_handler, Grape::Exceptions::InvalidVersionHeader, nil)
518+
expect(handler).to be_nil
519+
end
520+
end
491521
end

spec/grape/middleware/stack_spec.rb

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def initialize(&block)
1818
let(:others) { [[:use, bar_middleware], [:insert_before, bar_middleware, block_middleware, proc]] }
1919

2020
before do
21-
subject.use foo_middleware
21+
subject.use foo_middleware if subject.respond_to?(:use)
2222
end
2323

2424
describe '#use' do
@@ -151,4 +151,14 @@ def initialize(&block)
151151
subject.concat others
152152
end
153153
end
154+
155+
describe Grape::Middleware::Stack::Middleware do
156+
subject { described_class.new(foo_middleware, [], nil) }
157+
158+
describe '#==' do
159+
it 'returns falsy for an object that is neither a Middleware nor a Class' do
160+
expect(subject == 'not a middleware').to be_falsy
161+
end
162+
end
163+
end
154164
end

spec/grape/middleware/versioner/header_spec.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,4 +338,13 @@ def app
338338
expect { versioned_get '/', 'v1', using: :header }.to raise_error Grape::Exceptions::MissingVendorOption
339339
end
340340
end
341+
342+
context 'when the vendor option is not set on the middleware directly' do
343+
subject { described_class.new(app, version_options: Grape::DSL::VersionOptions.new(using: :header, vendor: nil)) }
344+
345+
it 'skips the best-quality-media-type match entirely' do
346+
status, = subject.call('HTTP_ACCEPT' => 'application/vnd.vendor+json')
347+
expect(status).to eq(200)
348+
end
349+
end
341350
end

spec/grape/namespace_spec.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,15 @@
3030
expect(namespace.hash).not_to eq(other.hash)
3131
end
3232
end
33+
34+
describe '.joined_space' do
35+
it 'maps a list of Namespace objects to their #space' do
36+
other = described_class.new('bar')
37+
expect(described_class.joined_space([namespace, other])).to eq(%w[foo bar])
38+
end
39+
40+
it 'returns nil for a nil settings list' do
41+
expect(described_class.joined_space(nil)).to be_nil
42+
end
43+
end
3344
end
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# frozen_string_literal: true
2+
3+
describe Grape::Router::BaseRoute do
4+
let(:pattern) { instance_double(Grape::Router::Pattern) }
5+
6+
describe '#initialize' do
7+
context 'when options is a plain Hash' do
8+
subject(:route) { described_class.new(pattern, { foo: 'bar' }) }
9+
10+
it 'wraps it in an ActiveSupport::OrderedOptions' do
11+
expect(route.options).to be_a(ActiveSupport::OrderedOptions)
12+
end
13+
14+
it 'reads back the given options' do
15+
expect(route.options[:foo]).to eq('bar')
16+
end
17+
end
18+
19+
context 'when options is already an ActiveSupport::OrderedOptions' do
20+
subject(:route) { described_class.new(pattern, options) }
21+
22+
let(:options) { ActiveSupport::OrderedOptions.new.update(foo: 'bar') }
23+
24+
it 'uses it as-is, without wrapping it again' do
25+
expect(route.options).to equal(options)
26+
end
27+
end
28+
end
29+
end

0 commit comments

Comments
 (0)