From 99457f15722b3326ba11ecb3003c3dd50b70a9c4 Mon Sep 17 00:00:00 2001 From: Robin Houston Date: Mon, 20 Aug 2012 08:00:03 +0100 Subject: Fix tests Also make the InfoRequest#is_old_unclassified? method a little more conservative, by returning false only is the is_external? method returns true. This makes it subtly inconsistent with InfoRequest.find_old_unclassified, but it is better I think to be subtly inconsistent than to risk breaking things that used to work. --- spec/models/info_request_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'spec/models') diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index 98ee34381..41d01c89a 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -353,7 +353,7 @@ describe InfoRequest do InfoRequest.should_receive(:find).with(:all, {:select=> anything, :order=> anything, - :conditions=>["awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen' and prominence != 'backpage'", + :conditions=>["awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen' and user_id is not null and prominence != 'backpage'", true, Time.now - 21.days]}) InfoRequest.find_old_unclassified({:conditions => ["prominence != 'backpage'"]}) end @@ -362,7 +362,7 @@ describe InfoRequest do InfoRequest.should_receive(:find).with(:all, {:select=>"*, (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) as last_response_time", :order=>"last_response_time", - :conditions=>["awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen'", + :conditions=>["awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen' and user_id is not null", true, Time.now - 21.days]}) InfoRequest.find_old_unclassified end @@ -396,7 +396,7 @@ describe InfoRequest do end it 'should return true if it is awaiting description, isn\'t the holding pen and hasn\'t had an event in 21 days' do - @info_request.is_old_unclassified?.should be_true + (@info_request.is_external? || @info_request.is_old_unclassified?).should be_true end end -- cgit v1.2.3 From ba97fffe11b02275a9b3619716744dca7fe9580b Mon Sep 17 00:00:00 2001 From: Robin Houston Date: Mon, 20 Aug 2012 08:18:03 +0100 Subject: Fix another test Is there actually any point in tests of this sort? It is not testing that anything works as intended, only that a particular SQL string is passed to the database -- and surely the only way that string could have been discovered for test purposes was essentially to copy it from the code under test. It seems to me that the only real function of a test of this sort is to make it more difficult to modify the code under test. --- spec/models/request_mailer_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/request_mailer_spec.rb b/spec/models/request_mailer_spec.rb index 64ac35cf7..ea75ec765 100644 --- a/spec/models/request_mailer_spec.rb +++ b/spec/models/request_mailer_spec.rb @@ -223,7 +223,7 @@ describe RequestMailer, "when sending reminders to requesters to classify a resp end it 'should ask for all requests that are awaiting description and whose latest response is older than the number of days given and that are not the holding pen' do - expected_params = {:conditions => [ "awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen'", + expected_params = {:conditions => [ "awaiting_description = ? and (select created_at from info_request_events where info_request_events.info_request_id = info_requests.id and info_request_events.event_type = 'response' order by created_at desc limit 1) < ? and url_title != 'holding_pen' and user_id is not null", true, Time.now() - 7.days ], :include => [ :user ], :order => "info_requests.id"} -- cgit v1.2.3