From ce1f7433f6821541f2a35ee5f04e37451bcadb1d Mon Sep 17 00:00:00 2001 From: Mak Inada Date: Sat, 24 Jan 2015 02:19:12 +0000 Subject: [PATCH 1/5] Added version to rspec since specs don't run with RSpec 3. --- kodama.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kodama.gemspec b/kodama.gemspec index b73c315..ec134a7 100644 --- a/kodama.gemspec +++ b/kodama.gemspec @@ -20,7 +20,7 @@ Gem::Specification.new do |gem| gem.add_dependency 'ruby-binlog', '>= 0.1.9' gem.add_development_dependency 'rake' - gem.add_development_dependency 'rspec' + gem.add_development_dependency 'rspec', '~> 2.0' gem.add_development_dependency 'pry' gem.add_development_dependency 'pry-nav' gem.add_development_dependency 'guard-rspec', '2.1.2' From d5741b329ce91aed88260a3e7f0005beb8528668 Mon Sep 17 00:00:00 2001 From: Mak Inada Date: Sat, 24 Jan 2015 02:23:59 +0000 Subject: [PATCH 2/5] Replaced stub_binlog_client with Rspec's late binding variables. No semantic change. --- spec/lib/client_spec.rb | 88 ++++++++++++++++++++++++----------------- 1 file changed, 51 insertions(+), 37 deletions(-) diff --git a/spec/lib/client_spec.rb b/spec/lib/client_spec.rb index 7729f1c..2e244fa 100644 --- a/spec/lib/client_spec.rb +++ b/spec/lib/client_spec.rb @@ -54,15 +54,18 @@ def read end end - def stub_binlog_client(events = [], connect = true) - client.stub(:binlog_client).and_return { TestBinlogClient.new(events, connect) } - end + let(:connect) { true } + let(:binlog_client) { TestBinlogClient.new(events, connect) } def stub_position_file(position_file = nil) client.stub(:position_file).and_return { position_file || TestPositionFile.new } end - let(:client) { Kodama::Client.new('mysql://user@host') } + let(:client) { + c = Kodama::Client.new('mysql://user@host') + c.stub(:binlog_client).and_return { binlog_client } + c + } let(:rotate_event) do mock(Binlog::RotateEvent).tap do |event| @@ -90,49 +93,60 @@ def stub_position_file(position_file = nil) end end - it 'should receive query_event' do - stub_binlog_client([query_event]) - expect {|block| - client.on_query_event(&block) - client.start - }.to yield_with_args(query_event) + context "with query event" do + let(:events) { [query_event] } + it 'should receive query_event' do + expect {|block| + client.on_query_event(&block) + client.start + }.to yield_with_args(query_event) + end end - it 'should receive row_event' do - stub_binlog_client([row_event]) - expect {|block| - client.on_row_event(&block) - client.start - }.to yield_with_args(row_event) + context "with row event" do + let(:events) { [row_event] } + it 'should receive row_event' do + expect {|block| + client.on_row_event(&block) + client.start + }.to yield_with_args(row_event) + end end - it 'should save position only on row, query and rotate event' do - stub_binlog_client([rotate_event, query_event, row_event, xid_event]) - position_file = TestPositionFile.new.tap do |pf| - pf.should_receive(:update).with('binlog', 100).once.ordered - pf.should_receive(:update).with('binlog', 200).once.ordered - pf.should_receive(:update).with('binlog', 300).once.ordered + context "with multiple events" do + let(:events) { [rotate_event, query_event, row_event, xid_event] } + it 'should save position only on row, query and rotate event' do + position_file = TestPositionFile.new.tap do |pf| + pf.should_receive(:update).with('binlog', 100).once.ordered + pf.should_receive(:update).with('binlog', 200).once.ordered + pf.should_receive(:update).with('binlog', 300).once.ordered + end + stub_position_file(position_file) + client.binlog_position_file = 'test' + client.start end - stub_position_file(position_file) - client.binlog_position_file = 'test' - client.start end - it 'should retry exactly specifeid times' do - stub_binlog_client([query_event], false) - client.connection_retry_limit = 2 - client.connection_retry_wait = 0.1 - expect { client.start }.to raise_error(Binlog::Error) - client.connection_retry_count.should == 2 + context "when an error occurred" do + let(:events) { [query_event] } + let(:connect) { false } + it 'should retry exactly specifeid times' do + client.connection_retry_limit = 2 + client.connection_retry_wait = 0.1 + expect { client.start }.to raise_error(Binlog::Error) + client.connection_retry_count.should == 2 + end end - it 'should stop when it receives stop request' do - stub_binlog_client([query_event, row_event]) - client.on_query_event do |event| - self.stop_request + context "with stop request" do + let(:events) { [query_event, row_event] } + it 'should stop when it receives stop request' do + client.on_query_event do |event| + self.stop_request + end + expect {|block| client.on_row_event(&block) }.not_to yield_control + client.start end - expect {|block| client.on_row_event(&block) }.not_to yield_control - client.start end end end From d401fab248c63da723a95d97c60d6f80ab352ecd Mon Sep 17 00:00:00 2001 From: Mak Inada Date: Sat, 24 Jan 2015 02:28:25 +0000 Subject: [PATCH 3/5] Added a spec which causes infinite retries. --- spec/lib/client_spec.rb | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/spec/lib/client_spec.rb b/spec/lib/client_spec.rb index 2e244fa..8dc6b32 100644 --- a/spec/lib/client_spec.rb +++ b/spec/lib/client_spec.rb @@ -127,7 +127,7 @@ def stub_position_file(position_file = nil) end end - context "when an error occurred" do + context "when connection failed" do let(:events) { [query_event] } let(:connect) { false } it 'should retry exactly specifeid times' do @@ -138,6 +138,20 @@ def stub_position_file(position_file = nil) end end + context "when an error is raised" do + let(:events) { [query_event] } + before do + client.connection_retry_limit = 2 + client.connection_retry_wait = 0.1 + + binlog_client.should_receive(:wait_for_next_event).and_raise Binlog::Error + end + it 'should retry exactly specifeid times' do + expect { client.start }.to raise_error(Binlog::Error) + client.connection_retry_count.should == 2 + end + end + context "with stop request" do let(:events) { [query_event, row_event] } it 'should stop when it receives stop request' do From e09fcbc49e50c243f7d6042814a8a5dacb0034c6 Mon Sep 17 00:00:00 2001 From: Mak Inada Date: Sat, 24 Jan 2015 02:38:09 +0000 Subject: [PATCH 4/5] Added call count check to prevent inifite loop. The spec still fails. --- spec/lib/client_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/lib/client_spec.rb b/spec/lib/client_spec.rb index 8dc6b32..7d3d9af 100644 --- a/spec/lib/client_spec.rb +++ b/spec/lib/client_spec.rb @@ -140,15 +140,15 @@ def stub_position_file(position_file = nil) context "when an error is raised" do let(:events) { [query_event] } + let(:retry_limit) { 2 } before do - client.connection_retry_limit = 2 + client.connection_retry_limit = retry_limit client.connection_retry_wait = 0.1 - - binlog_client.should_receive(:wait_for_next_event).and_raise Binlog::Error end it 'should retry exactly specifeid times' do + binlog_client.should_receive(:wait_for_next_event).exactly(retry_limit + 1).times.and_raise(Binlog::Error) expect { client.start }.to raise_error(Binlog::Error) - client.connection_retry_count.should == 2 + client.connection_retry_count.should == retry_limit end end From 9f0d1fb9e3232282ee44c82aa318b08bca4f688d Mon Sep 17 00:00:00 2001 From: Mak Inada Date: Sat, 24 Jan 2015 02:38:56 +0000 Subject: [PATCH 5/5] Moved @retry_info.count_reset out of the retry loop. --- lib/kodama/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/kodama/client.rb b/lib/kodama/client.rb index 6e6aa38..9fe8f33 100644 --- a/lib/kodama/client.rb +++ b/lib/kodama/client.rb @@ -99,10 +99,10 @@ def stop_request end def start + @retry_info.count_reset begin client = binlog_client(@url) raise Binlog::Error, 'MySQL server has gone away' unless client.connect - @retry_info.count_reset if @binlog_info.valid? client.set_position(@binlog_info.filename, @binlog_info.position)