From 2cdd76b23d4604b85aecdbc33cb57fd240f43b3a Mon Sep 17 00:00:00 2001 From: Jeff Moore Date: Tue, 1 May 2018 14:44:43 -0700 Subject: [PATCH 01/10] Backfill OAuth2::Error specs A start at attempting to provide coverage of the non-StandardError behavior that this class implements. Also, no longer rely on a local being defined inside of an `if`. --- lib/oauth2/error.rb | 6 ++- spec/oauth2/error_spec.rb | 106 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 spec/oauth2/error_spec.rb diff --git a/lib/oauth2/error.rb b/lib/oauth2/error.rb index b77355f3..09fdd996 100644 --- a/lib/oauth2/error.rb +++ b/lib/oauth2/error.rb @@ -7,11 +7,13 @@ class Error < StandardError def initialize(response) response.error = self @response = response + error_description = '' if response.parsed.is_a?(Hash) @code = response.parsed['error'] @description = response.parsed['error_description'] - error_description = "#{@code}: #{@description}" + error_description << "#{@code}: " if @code + error_description << @description if @description end super(error_message(response.body, :error_description => error_description)) @@ -23,7 +25,7 @@ def initialize(response) def error_message(response_body, opts = {}) message = [] - opts[:error_description] && message << opts[:error_description] + message << opts[:error_description] unless opts[:error_description].empty? error_message = if opts[:error_description] && opts[:error_description].respond_to?(:encoding) script_encoding = opts[:error_description].encoding diff --git a/spec/oauth2/error_spec.rb b/spec/oauth2/error_spec.rb new file mode 100644 index 00000000..08ebec55 --- /dev/null +++ b/spec/oauth2/error_spec.rb @@ -0,0 +1,106 @@ +RSpec.describe OAuth2::Error do + let(:subject) { described_class.new(response) } + let(:response) do + fake_response = double( + 'response', + :status => 418, + :headers => response_headers, + :body => response_body, + ) + + OAuth2::Response.new(fake_response) + end + + let(:response_headers) { {'Content-Type' => 'application/json'} } + let(:response_body) { {:text => 'Coffee brewing failed'}.to_json } + + it 'sets self to #error on the response object' do + expect(response.error).to be_nil + error = described_class.new(response) + expect(response.error).to equal(error) + end + + it 'sets the response object to #response on self' do + error = described_class.new(response) + expect(error.response).to equal(response) + end + + describe 'attr_readers' do + it 'has code' do + expect(subject).to respond_to(:code) + end + + it 'has description' do + expect(subject).to respond_to(:description) + end + + it 'has response' do + expect(subject).to respond_to(:response) + end + end + + context 'when the response is parseable as a hash' do + let(:response_body) { response_hash.to_json } + let(:response_hash) { {:text => 'Coffee brewing failed'} } + + context 'when the response has an error and error_description' do + before do + response_hash[:error_description] = 'Short and stout' + response_hash[:error] = 'i_am_a_teapot' + end + + it 'prepends to the error message with a return character' do + expect(subject.message.lines).to eq([ + 'i_am_a_teapot: Short and stout' + "\n", + '{"text":"Coffee brewing failed","error_description":"Short and stout","error":"i_am_a_teapot"}' + ]) + end + + it 'sets the code attribute' do + expect(subject.code).to eq('i_am_a_teapot') + end + + it 'sets the description attribute' do + expect(subject.description).to eq('Short and stout') + end + end + + context 'when there is no error description' do + before do + expect(response_hash).to_not have_key(:error) + expect(response_hash).to_not have_key(:error_description) + end + + it 'does not prepend anything to the message' do + expect(subject.message.lines.count).to eq(1) + expect(subject.message).to eq '{"text":"Coffee brewing failed"}' + end + + it 'does not set code' do + expect(subject.code).to be_nil + end + + it 'does not set description' do + expect(subject.description).to be_nil + end + end + end + + context 'when the response does not parse to a hash' do + let(:response_headers) { {'Content-Type' => 'text/html'} } + let(:response_body) { 'Hello, I am a teapot' } + + it 'does not do anything to the message' do + expect(subject.message.lines.count).to eq(1) + expect(subject.message).to eq(response_body) + end + + it 'does not set code' do + expect(subject.code).to be_nil + end + + it 'does not set description' do + expect(subject.description).to be_nil + end + end +end From 82bbaea374435d34900108895c13a5714d043dc7 Mon Sep 17 00:00:00 2001 From: Jeff Moore Date: Tue, 1 May 2018 16:02:44 -0700 Subject: [PATCH 02/10] Fix encoding error when encoding an error If the response body is an object that does not respond to `encode`, don't try to encode it. Add spec coverage for a UTF-8 character being returned in a response. Additionally, make `error_message` into a private method, since no other file calls it. --- .rubocop_rspec.yml | 3 ++ lib/oauth2/error.rb | 42 ++++++++++++++++----------- spec/oauth2/error_spec.rb | 60 ++++++++++++++++++++++++++++----------- 3 files changed, 72 insertions(+), 33 deletions(-) diff --git a/.rubocop_rspec.yml b/.rubocop_rspec.yml index 4e366ed4..347777dc 100644 --- a/.rubocop_rspec.yml +++ b/.rubocop_rspec.yml @@ -21,3 +21,6 @@ RSpec/InstanceVariable: RSpec/NestedGroups: Enabled: false + +RSpec/ExpectInHook: + Enabled: false diff --git a/lib/oauth2/error.rb b/lib/oauth2/error.rb index 09fdd996..3534d254 100644 --- a/lib/oauth2/error.rb +++ b/lib/oauth2/error.rb @@ -7,36 +7,44 @@ class Error < StandardError def initialize(response) response.error = self @response = response - error_description = '' + message_opts = {} if response.parsed.is_a?(Hash) @code = response.parsed['error'] @description = response.parsed['error_description'] - error_description << "#{@code}: " if @code - error_description << @description if @description + message_opts = parse_error_description(@code, @description) end - super(error_message(response.body, :error_description => error_description)) + super(error_message(response.body, message_opts)) end - # Makes a error message - # @param [String] response_body response body of request - # @param [String] opts :error_description error description to show first line + private + def error_message(response_body, opts = {}) - message = [] + lines = [] - message << opts[:error_description] unless opts[:error_description].empty? + lines << opts[:error_description] if opts[:error_description] - error_message = if opts[:error_description] && opts[:error_description].respond_to?(:encoding) - script_encoding = opts[:error_description].encoding - response_body.encode(script_encoding, :invalid => :replace, :undef => :replace) - else - response_body - end + error_string = if response_body.respond_to?(:encoding) && opts[:error_description].respond_to?(:encoding) + script_encoding = opts[:error_description].encoding + response_body.encode(script_encoding, :invalid => :replace, :undef => :replace) + else + response_body + end - message << error_message + lines << error_string + + lines.join("\n") + end + + def parse_error_description(code, description) + return {} unless code || description + + error_description = '' + error_description << "#{code}: " if code + error_description << description if description - message.join("\n") + {:error_description => error_description} end end end diff --git a/spec/oauth2/error_spec.rb b/spec/oauth2/error_spec.rb index 08ebec55..88781fbc 100644 --- a/spec/oauth2/error_spec.rb +++ b/spec/oauth2/error_spec.rb @@ -2,12 +2,12 @@ let(:subject) { described_class.new(response) } let(:response) do fake_response = double( - 'response', - :status => 418, + 'response', + :status => 418, :headers => response_headers, - :body => response_body, + :body => response_body ) - + OAuth2::Response.new(fake_response) end @@ -19,21 +19,21 @@ error = described_class.new(response) expect(response.error).to equal(error) end - + it 'sets the response object to #response on self' do error = described_class.new(response) expect(error.response).to equal(response) end - + describe 'attr_readers' do it 'has code' do expect(subject).to respond_to(:code) end - + it 'has description' do expect(subject).to respond_to(:description) end - + it 'has response' do expect(subject).to respond_to(:response) end @@ -50,10 +50,25 @@ end it 'prepends to the error message with a return character' do - expect(subject.message.lines).to eq([ - 'i_am_a_teapot: Short and stout' + "\n", - '{"text":"Coffee brewing failed","error_description":"Short and stout","error":"i_am_a_teapot"}' - ]) + expect(subject.message.lines).to eq( + [ + 'i_am_a_teapot: Short and stout' + "\n", + '{"text":"Coffee brewing failed","error_description":"Short and stout","error":"i_am_a_teapot"}', + ] + ) + end + + context 'when the response needs to be encoded' do + let(:response_body) { MultiJson.encode(response_hash).force_encoding('ASCII-8BIT') } + + before do + response_hash[:error_description] << ": 'A magical voyage of tea 🍵'" + end + + it 'respects the encoding of the response body' do + expect(subject.message).to match(/🍵/) + # This will fail with Encoding::CompatibilityError if done incorrectly + end end it 'sets the code attribute' do @@ -67,8 +82,8 @@ context 'when there is no error description' do before do - expect(response_hash).to_not have_key(:error) - expect(response_hash).to_not have_key(:error_description) + expect(response_hash).not_to have_key(:error) + expect(response_hash).not_to have_key(:error_description) end it 'does not prepend anything to the message' do @@ -94,13 +109,26 @@ expect(subject.message.lines.count).to eq(1) expect(subject.message).to eq(response_body) end - + it 'does not set code' do expect(subject.code).to be_nil end - + it 'does not set description' do expect(subject.description).to be_nil end end + + describe 'message body encoding' do + context 'when the response is not an encodable thing' do + # i.e. a Ruby hash + + let(:response_headers) { {'Content-Type' => 'who knows'} } + let(:response_body) { {:text => 'Coffee brewing failed'} } + + it 'does not try to encode the message string' do + expect(subject.message).to eq(response_body.to_s) + end + end + end end From 9e41bd0467c9cbbe5ba020c9ab5cf64afdee4754 Mon Sep 17 00:00:00 2001 From: Jeff Moore Date: Wed, 2 May 2018 10:19:37 -0700 Subject: [PATCH 03/10] Check that response body responds to encode Not sure under what circumstances an object would respond to `encoding` and not `encode` but better safe than sorry. --- lib/oauth2/error.rb | 2 +- spec/oauth2/error_spec.rb | 31 ++++++++++++++++++------------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/lib/oauth2/error.rb b/lib/oauth2/error.rb index 3534d254..2eab7dfd 100644 --- a/lib/oauth2/error.rb +++ b/lib/oauth2/error.rb @@ -25,7 +25,7 @@ def error_message(response_body, opts = {}) lines << opts[:error_description] if opts[:error_description] - error_string = if response_body.respond_to?(:encoding) && opts[:error_description].respond_to?(:encoding) + error_string = if response_body.respond_to?(:encode) && opts[:error_description].respond_to?(:encoding) script_encoding = opts[:error_description].encoding response_body.encode(script_encoding, :invalid => :replace, :undef => :replace) else diff --git a/spec/oauth2/error_spec.rb b/spec/oauth2/error_spec.rb index 88781fbc..7d37d9ef 100644 --- a/spec/oauth2/error_spec.rb +++ b/spec/oauth2/error_spec.rb @@ -71,6 +71,20 @@ end end + context 'when the response is not an encodable thing' do + let(:response_headers) { {'Content-Type' => 'who knows'} } + let(:response_body) { {:text => 'Coffee brewing failed'} } + + before do + expect(response_body).not_to respond_to(:encode) + # i.e. a Ruby hash + end + + it 'does not try to encode the message string' do + expect(subject.message).to eq(response_body.to_s) + end + end + it 'sets the code attribute' do expect(subject.code).to eq('i_am_a_teapot') end @@ -104,6 +118,10 @@ context 'when the response does not parse to a hash' do let(:response_headers) { {'Content-Type' => 'text/html'} } let(:response_body) { 'Hello, I am a teapot' } + + before do + expect(response.parsed).not_to be_a(Hash) + end it 'does not do anything to the message' do expect(subject.message.lines.count).to eq(1) @@ -118,17 +136,4 @@ expect(subject.description).to be_nil end end - - describe 'message body encoding' do - context 'when the response is not an encodable thing' do - # i.e. a Ruby hash - - let(:response_headers) { {'Content-Type' => 'who knows'} } - let(:response_body) { {:text => 'Coffee brewing failed'} } - - it 'does not try to encode the message string' do - expect(subject.message).to eq(response_body.to_s) - end - end - end end From 4f1c68d21b2344bf92dcaf3ba0a3e92767c53fc2 Mon Sep 17 00:00:00 2001 From: Jeff Moore Date: Wed, 2 May 2018 11:21:01 -0700 Subject: [PATCH 04/10] Backfill tests for character encoding conversion The :invalid and :undef options to `encode` are definitely necessary, and nothing was really testing them. --- spec/oauth2/error_spec.rb | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/spec/oauth2/error_spec.rb b/spec/oauth2/error_spec.rb index 7d37d9ef..88086533 100644 --- a/spec/oauth2/error_spec.rb +++ b/spec/oauth2/error_spec.rb @@ -61,13 +61,26 @@ context 'when the response needs to be encoded' do let(:response_body) { MultiJson.encode(response_hash).force_encoding('ASCII-8BIT') } - before do - response_hash[:error_description] << ": 'A magical voyage of tea 🍵'" + context 'with invalid characters present' do + before do + response_body.gsub!('failed', "\255 invalid \255") + end + + it 'replaces them' do + expect(subject.message).to match(/� invalid �/) + # This will fail with Encoding::InvalidByteSequenceError if {:invalid => replace} is not passed in + end end - it 'respects the encoding of the response body' do - expect(subject.message).to match(/🍵/) - # This will fail with Encoding::CompatibilityError if done incorrectly + context 'with undefined characters present' do + before do + response_hash[:error_description] << ": 'A magical voyage of tea 🍵'" + end + + it 'replaces them' do + expect(subject.message).to match(/tea �/) + # This will fail with Encoding::CompatibilityError if {:undef => replace} is not passed in + end end end @@ -118,7 +131,7 @@ context 'when the response does not parse to a hash' do let(:response_headers) { {'Content-Type' => 'text/html'} } let(:response_body) { 'Hello, I am a teapot' } - + before do expect(response.parsed).not_to be_a(Hash) end From 313776d4d2a50a0b584ae11da614e74c47dfd573 Mon Sep 17 00:00:00 2001 From: Jeff Moore Date: Wed, 2 May 2018 13:32:51 -0700 Subject: [PATCH 05/10] Attempt to fix invalid byte sequence exceptions in JRuby Seems like RSpec matchers can blow up with garbage characters under some circumstances --- spec/oauth2/error_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/oauth2/error_spec.rb b/spec/oauth2/error_spec.rb index 88086533..55f3daa5 100644 --- a/spec/oauth2/error_spec.rb +++ b/spec/oauth2/error_spec.rb @@ -63,12 +63,12 @@ context 'with invalid characters present' do before do - response_body.gsub!('failed', "\255 invalid \255") + response_body.gsub!('stout', "\255 invalid \255") end it 'replaces them' do - expect(subject.message).to match(/� invalid �/) - # This will fail with Encoding::InvalidByteSequenceError if {:invalid => replace} is not passed in + expect(subject.message.include?("� invalid �")).to be_truthy + # This will fail if {:invalid => replace} is not passed into `encode` end end @@ -78,8 +78,8 @@ end it 'replaces them' do - expect(subject.message).to match(/tea �/) - # This will fail with Encoding::CompatibilityError if {:undef => replace} is not passed in + expect(subject.message.include?("tea �")).to be_truthy + # This will fail if {:undef => replace} is not passed into `encode` end end end From 89aa67f47ed69e888fa5f145c2d156f37808abf9 Mon Sep 17 00:00:00 2001 From: Jeff Moore Date: Wed, 2 May 2018 14:31:35 -0700 Subject: [PATCH 06/10] Rubocop does not like matcher workarounds --- spec/oauth2/error_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/oauth2/error_spec.rb b/spec/oauth2/error_spec.rb index 55f3daa5..452e8bb4 100644 --- a/spec/oauth2/error_spec.rb +++ b/spec/oauth2/error_spec.rb @@ -67,7 +67,7 @@ end it 'replaces them' do - expect(subject.message.include?("� invalid �")).to be_truthy + raise 'Invalid characters not replaced' unless subject.message.include?('� invalid �') # This will fail if {:invalid => replace} is not passed into `encode` end end @@ -78,7 +78,7 @@ end it 'replaces them' do - expect(subject.message.include?("tea �")).to be_truthy + raise 'Undefined characters not replaced' unless subject.message.include?('tea �') # This will fail if {:undef => replace} is not passed into `encode` end end From f212e6b4cfa90081d871f3ecf477094507ca9afc Mon Sep 17 00:00:00 2001 From: Jeff Moore Date: Fri, 4 May 2018 10:25:30 -0700 Subject: [PATCH 07/10] Skip invalid character encoding spec in lower ruby versions It seems like this has always been broken, but no spec was ever passing in an invalid byte sequence as a response message. The behavior of `encode` changed in 2.1, and support for these older rubies is about to be dropped anyways. --- Gemfile | 1 + spec/oauth2/error_spec.rb | 6 ++++++ spec/spec_helper.rb | 1 + 3 files changed, 8 insertions(+) diff --git a/Gemfile b/Gemfile index 91bbbf40..8a6e97d0 100644 --- a/Gemfile +++ b/Gemfile @@ -9,6 +9,7 @@ group :test do gem 'rubocop-rspec', '~> 1.24.0' end gem 'pry', '~> 0.11' if ruby_version >= Gem::Version.new('2.0') + gem 'rspec-pending_for' end # Specify non-special dependencies in oauth2.gemspec diff --git a/spec/oauth2/error_spec.rb b/spec/oauth2/error_spec.rb index 452e8bb4..ad4482e7 100644 --- a/spec/oauth2/error_spec.rb +++ b/spec/oauth2/error_spec.rb @@ -67,6 +67,12 @@ end it 'replaces them' do + # The skip can be removed once support for < 2.1 is dropped. + encoding_skip = {:versions => %w[1.8.7 1.9.3 2.0.0], :reason => 'encode/scrub only works as of Ruby 2.1'} + skip_for(encoding_skip.merge(:engine => 'ruby')) + skip_for(encoding_skip.merge(:engine => 'jruby')) + # See https://bibwild.wordpress.com/2013/03/12/removing-illegal-bytes-for-encoding-in-ruby-1-9-strings/ + raise 'Invalid characters not replaced' unless subject.message.include?('� invalid �') # This will fail if {:invalid => replace} is not passed into `encode` end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 70f64474..0618ddf9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,7 @@ require 'bundler/setup' require 'oauth2' require 'helper' +require 'rspec/pending_for' RSpec.configure do |config| # Enable flags like --only-failures and --next-failure From e11151f0bb7aff6bb956b82f97212e3548cdf229 Mon Sep 17 00:00:00 2001 From: Jeff Moore Date: Fri, 4 May 2018 11:03:06 -0700 Subject: [PATCH 08/10] Add rspec-pending_for to version gemfiles --- gemfiles/jruby_1.7.gemfile | 1 + gemfiles/jruby_9.0.gemfile | 1 + gemfiles/jruby_9.1.gemfile | 1 + gemfiles/jruby_head.gemfile | 1 + gemfiles/ruby_1.9.gemfile | 1 + gemfiles/ruby_2.0.gemfile | 1 + gemfiles/ruby_2.1.gemfile | 1 + gemfiles/ruby_2.2.gemfile | 1 + gemfiles/ruby_2.3.gemfile | 1 + gemfiles/ruby_2.4.gemfile | 1 + gemfiles/ruby_2.5.gemfile | 1 + gemfiles/ruby_head.gemfile | 1 + 12 files changed, 12 insertions(+) diff --git a/gemfiles/jruby_1.7.gemfile b/gemfiles/jruby_1.7.gemfile index e3f9afd5..f31532b0 100644 --- a/gemfiles/jruby_1.7.gemfile +++ b/gemfiles/jruby_1.7.gemfile @@ -11,6 +11,7 @@ group :test do # gem "rubocop", "~> 0.41.2" gem 'rake' gem 'rspec' + gem 'rspec-pending_for' end gemspec :path => '../' diff --git a/gemfiles/jruby_9.0.gemfile b/gemfiles/jruby_9.0.gemfile index e924355b..9cd4d582 100644 --- a/gemfiles/jruby_9.0.gemfile +++ b/gemfiles/jruby_9.0.gemfile @@ -3,6 +3,7 @@ source 'https://rubygems.org' group :test do gem 'rake' gem 'rspec' + gem 'rspec-pending_for' gem 'rubocop', '~> 0.53.0' gem 'rubocop-rspec', '~> 1.24.0' end diff --git a/gemfiles/jruby_9.1.gemfile b/gemfiles/jruby_9.1.gemfile index 920dabea..60080c62 100644 --- a/gemfiles/jruby_9.1.gemfile +++ b/gemfiles/jruby_9.1.gemfile @@ -7,6 +7,7 @@ end group :test do gem 'rake' gem 'rspec' + gem 'rspec-pending_for' gem 'rubocop', '~> 0.53.0' gem 'rubocop-rspec', '~> 1.24.0' end diff --git a/gemfiles/jruby_head.gemfile b/gemfiles/jruby_head.gemfile index 671290d6..64436169 100644 --- a/gemfiles/jruby_head.gemfile +++ b/gemfiles/jruby_head.gemfile @@ -9,6 +9,7 @@ end group :test do gem 'rake' gem 'rspec' + gem 'rspec-pending_for' # rubocop:disable Bundler/DuplicatedGem gem 'rubocop', '~> 0.53.0' gem 'rubocop-rspec', '~> 1.24.0' diff --git a/gemfiles/ruby_1.9.gemfile b/gemfiles/ruby_1.9.gemfile index 49c0a414..369c14a6 100644 --- a/gemfiles/ruby_1.9.gemfile +++ b/gemfiles/ruby_1.9.gemfile @@ -8,6 +8,7 @@ gem 'tins', '< 1.7' group :test do gem 'rake' gem 'rspec' + gem 'rspec-pending_for' end gemspec :path => '../' diff --git a/gemfiles/ruby_2.0.gemfile b/gemfiles/ruby_2.0.gemfile index 6244eb2f..8fba443a 100644 --- a/gemfiles/ruby_2.0.gemfile +++ b/gemfiles/ruby_2.0.gemfile @@ -9,6 +9,7 @@ end group :test do gem 'rake' gem 'rspec' + gem 'rspec-pending_for' end gemspec :path => '../' diff --git a/gemfiles/ruby_2.1.gemfile b/gemfiles/ruby_2.1.gemfile index 20f56b70..41632192 100644 --- a/gemfiles/ruby_2.1.gemfile +++ b/gemfiles/ruby_2.1.gemfile @@ -9,6 +9,7 @@ end group :test do gem 'rake' gem 'rspec' + gem 'rspec-pending_for' gem 'rubocop', '~> 0.53.0' gem 'rubocop-rspec', '~> 1.24.0' end diff --git a/gemfiles/ruby_2.2.gemfile b/gemfiles/ruby_2.2.gemfile index 920dabea..60080c62 100644 --- a/gemfiles/ruby_2.2.gemfile +++ b/gemfiles/ruby_2.2.gemfile @@ -7,6 +7,7 @@ end group :test do gem 'rake' gem 'rspec' + gem 'rspec-pending_for' gem 'rubocop', '~> 0.53.0' gem 'rubocop-rspec', '~> 1.24.0' end diff --git a/gemfiles/ruby_2.3.gemfile b/gemfiles/ruby_2.3.gemfile index 920dabea..60080c62 100644 --- a/gemfiles/ruby_2.3.gemfile +++ b/gemfiles/ruby_2.3.gemfile @@ -7,6 +7,7 @@ end group :test do gem 'rake' gem 'rspec' + gem 'rspec-pending_for' gem 'rubocop', '~> 0.53.0' gem 'rubocop-rspec', '~> 1.24.0' end diff --git a/gemfiles/ruby_2.4.gemfile b/gemfiles/ruby_2.4.gemfile index 920dabea..60080c62 100644 --- a/gemfiles/ruby_2.4.gemfile +++ b/gemfiles/ruby_2.4.gemfile @@ -7,6 +7,7 @@ end group :test do gem 'rake' gem 'rspec' + gem 'rspec-pending_for' gem 'rubocop', '~> 0.53.0' gem 'rubocop-rspec', '~> 1.24.0' end diff --git a/gemfiles/ruby_2.5.gemfile b/gemfiles/ruby_2.5.gemfile index 920dabea..60080c62 100644 --- a/gemfiles/ruby_2.5.gemfile +++ b/gemfiles/ruby_2.5.gemfile @@ -7,6 +7,7 @@ end group :test do gem 'rake' gem 'rspec' + gem 'rspec-pending_for' gem 'rubocop', '~> 0.53.0' gem 'rubocop-rspec', '~> 1.24.0' end diff --git a/gemfiles/ruby_head.gemfile b/gemfiles/ruby_head.gemfile index 920dabea..60080c62 100644 --- a/gemfiles/ruby_head.gemfile +++ b/gemfiles/ruby_head.gemfile @@ -7,6 +7,7 @@ end group :test do gem 'rake' gem 'rspec' + gem 'rspec-pending_for' gem 'rubocop', '~> 0.53.0' gem 'rubocop-rspec', '~> 1.24.0' end From 489eef39b75d4d3150292f9ac1aed517baca7527 Mon Sep 17 00:00:00 2001 From: Jeff Moore Date: Fri, 4 May 2018 11:18:15 -0700 Subject: [PATCH 09/10] Try to get multi-byte character parsing working on 1.9.3 Also try to skip invalid character encoding spec in all JRuby --- spec/oauth2/error_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/oauth2/error_spec.rb b/spec/oauth2/error_spec.rb index ad4482e7..c71d6611 100644 --- a/spec/oauth2/error_spec.rb +++ b/spec/oauth2/error_spec.rb @@ -1,3 +1,4 @@ +# encoding: UTF-8 RSpec.describe OAuth2::Error do let(:subject) { described_class.new(response) } let(:response) do @@ -68,9 +69,9 @@ it 'replaces them' do # The skip can be removed once support for < 2.1 is dropped. - encoding_skip = {:versions => %w[1.8.7 1.9.3 2.0.0], :reason => 'encode/scrub only works as of Ruby 2.1'} - skip_for(encoding_skip.merge(:engine => 'ruby')) - skip_for(encoding_skip.merge(:engine => 'jruby')) + encoding = {:reason => 'encode/scrub only works as of Ruby 2.1'} + skip_for(encoding.merge(:engine => 'ruby', :versions => %w[1.8.7 1.9.3 2.0.0])) + skip_for(encoding.merge(:engine => 'jruby')) # See https://bibwild.wordpress.com/2013/03/12/removing-illegal-bytes-for-encoding-in-ruby-1-9-strings/ raise 'Invalid characters not replaced' unless subject.message.include?('� invalid �') From 307382987487299624b4df50651def21211a5580 Mon Sep 17 00:00:00 2001 From: Jeff Moore Date: Fri, 4 May 2018 15:17:51 -0700 Subject: [PATCH 10/10] Fix multi-line message assertion for old Ruby versions Turns out String#lines returns an enumerator in 1.9.3 and an array in most everything else. --- spec/oauth2/error_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/oauth2/error_spec.rb b/spec/oauth2/error_spec.rb index c71d6611..85122b6f 100644 --- a/spec/oauth2/error_spec.rb +++ b/spec/oauth2/error_spec.rb @@ -1,4 +1,5 @@ # encoding: UTF-8 + RSpec.describe OAuth2::Error do let(:subject) { described_class.new(response) } let(:response) do @@ -51,7 +52,7 @@ end it 'prepends to the error message with a return character' do - expect(subject.message.lines).to eq( + expect(subject.message.each_line.to_a).to eq( [ 'i_am_a_teapot: Short and stout' + "\n", '{"text":"Coffee brewing failed","error_description":"Short and stout","error":"i_am_a_teapot"}',