From 84efe7ce1ed2a9050d5c5f81d7c9feed33bfc5de Mon Sep 17 00:00:00 2001 From: Ethan Vizitei Date: Tue, 23 Oct 2012 16:08:56 -0500 Subject: [PATCH] [#5773] Ruby 1.9.3 spec fixes [#5773] correctly evaluate the custom_fields_string My impression is that order is not enforced in the hash being used to build the fields string, as this was failing in 1.8.7 for me (but not in 1.9.3), because the string being checked was "b=456\na=123" while the string being returned from the custom_fields_string method was "a=123\nb=456". Testing with a regex instead confirms the content is there without relying on any particular order. [EDIT: Per Brian P, decided to sort the params instead to enforce order] [#5773] fix admin_settings_tab_spec for ruby 1.9.3 String's "each" method was being used, which is gone in 1.9. Fixed by performing a guard clause in the check_box_verifier in the spec file to conditionally take a string argument and convert it to an array. Also converted the rest of the method to assume an array value for that argument. [#5773] fix collaborations selenium spec for 1.9.3 This is an instance of using the "each" method of the string class which is no longer present in 1.9 in one of the helper methods in this spec. Erected a guard clause to check for Array-ness and to replace the argument with an array if necessary. [#5773] fix courses show view There was a spot in this view where it was interpolating "self.id" into an html attribute rather than "@instance_model.id". This sort of worked in ruby 1.8, where it would interpolate the object id of the view instance. In 1.9, that id becomes "object_id", which is what one would have to change the code to in order to preserve the current behavior. However, it seems more correct to simply pull the correct id of the contextual model object instead (which is what I did). [#5773] fix teacher_wiki_and_tiny_images spec This is an instance of using a string as an enumerable; adding a guard clause to lib/sticky_sis_fields.rb to transform the string argument into an array fixes it [#5773] fix selenium/student_submissions_spec.rb This is an issue with SSLCommon's use of net/https. In older ruby versions there are 2 parameters returned (the response, and the body). Now only the response is returned, so the response object has to be asked for the body explicitly. This fix was made in spec/selenium/helpers/files_common.rb Change-Id: I66a25ebbc1d8e5af7b7117375210172d287a77c7 Reviewed-on: https://gerrit.instructure.com/14641 Tested-by: Jenkins Reviewed-by: Brian Palmer --- app/models/context_external_tool.rb | 2 +- app/views/courses/show.html.erb | 2 +- lib/sticky_sis_fields.rb | 1 + spec/models/context_external_tool_spec.rb | 3 ++- spec/selenium/admin/admin_settings_tab_spec.rb | 5 ++++- spec/selenium/collaborations_spec.rb | 1 + spec/selenium/helpers/files_common.rb | 15 +++++++++------ 7 files changed, 19 insertions(+), 10 deletions(-) diff --git a/app/models/context_external_tool.rb b/app/models/context_external_tool.rb index 266d24c355c..71e0263b48c 100644 --- a/app/models/context_external_tool.rb +++ b/app/models/context_external_tool.rb @@ -107,7 +107,7 @@ class ContextExternalTool < ActiveRecord::Base def custom_fields_string (settings[:custom_fields] || {}).map{|key, val| "#{key}=#{val}" - }.join("\n") + }.sort.join("\n") end def config_type=(val) diff --git a/app/views/courses/show.html.erb b/app/views/courses/show.html.erb index 00bbb7b22fc..d9254c89acd 100644 --- a/app/views/courses/show.html.erb +++ b/app/views/courses/show.html.erb @@ -29,7 +29,7 @@ <%= t('#buttons.cancel', %{Cancel}) %> - <% elsif @context_enrollment && @context_enrollment.self_enrolled && @context_enrollment.active? && (!session["role_course_#{self.id}"]) %> + <% elsif @context_enrollment && @context_enrollment.self_enrolled && @context_enrollment.active? && (!session["role_course_#{@context.id}"]) %>
<%= image_tag "delete_circle.png" %> <%= t('links.drop_course', %{Drop this Course}) %>
diff --git a/lib/sticky_sis_fields.rb b/lib/sticky_sis_fields.rb index 2d5238e2d06..feaeeac1208 100644 --- a/lib/sticky_sis_fields.rb +++ b/lib/sticky_sis_fields.rb @@ -49,6 +49,7 @@ module StickySisFields end def stuck_sis_fields=(fields) + fields = [fields] if (fields.is_a? String) clear_sis_stickiness(*(stuck_sis_fields.to_a)) add_sis_stickiness(*(fields.map(&:to_sym).to_set)) end diff --git a/spec/models/context_external_tool_spec.rb b/spec/models/context_external_tool_spec.rb index 16311ec7ba8..63bf8e5997c 100644 --- a/spec/models/context_external_tool_spec.rb +++ b/spec/models/context_external_tool_spec.rb @@ -274,7 +274,8 @@ describe ContextExternalTool do it "should return custom_fields_string as a text-formatted field" do tool = @course.context_external_tools.create!(:name => "a", :url => "http://www.google.com", :consumer_key => '12345', :shared_secret => 'secret', :custom_fields => {'a' => '123', 'b' => '456'}) - tool.custom_fields_string.should == "a=123\nb=456" + fields_string = tool.custom_fields_string + fields_string.should == "a=123\nb=456" end it "should merge custom fields for extension launches" do diff --git a/spec/selenium/admin/admin_settings_tab_spec.rb b/spec/selenium/admin/admin_settings_tab_spec.rb index f94bb5cd99e..6f02bee76e4 100644 --- a/spec/selenium/admin/admin_settings_tab_spec.rb +++ b/spec/selenium/admin/admin_settings_tab_spec.rb @@ -24,6 +24,9 @@ describe "admin settings tab" do def check_box_verifier (css_selectors, features, checker = true) is_symbol = false + + css_selectors = [css_selectors] unless (css_selectors.is_a? Array) + if features.is_a? Symbol is_symbol = true if features == :all_selectors @@ -57,7 +60,7 @@ describe "admin settings tab" do default_selectors.push("#account_services_#{feature.to_s}") end if (checker) - default_selectors.push css_selectors + default_selectors += css_selectors end css_selectors = default_selectors else diff --git a/spec/selenium/collaborations_spec.rb b/spec/selenium/collaborations_spec.rb index 347494c5505..03b898f1f14 100644 --- a/spec/selenium/collaborations_spec.rb +++ b/spec/selenium/collaborations_spec.rb @@ -17,6 +17,7 @@ describe "collaborations" do end def go_to_collaboration_and_validate(urls = %W(/courses/#{@course.id}/collaborations), wizard_visible = true, execute_script = false) + urls = [urls] unless (urls.is_a? Array) urls.each { |url| get url } wait_for_ajaximations driver.execute_script "window.confirm = function(msg) { return true; }" if execute_script diff --git a/spec/selenium/helpers/files_common.rb b/spec/selenium/helpers/files_common.rb index 78dff050516..1d750f0f7ef 100644 --- a/spec/selenium/helpers/files_common.rb +++ b/spec/selenium/helpers/files_common.rb @@ -9,10 +9,10 @@ shared_examples_for "files selenium shared" do end def login(username, password) - resp, body = SSLCommon.get "#{app_host}/login" + resp = SSLCommon.get "#{app_host}/login" resp.code.should == "200" @cookie = resp.response['set-cookie'] - resp, body = SSLCommon.post_form("#{app_host}/login", { + resp = SSLCommon.post_form("#{app_host}/login", { "pseudonym_session[unique_id]" => username, "pseudonym_session[password]" => password, "redirect_to_ssl" => "0", @@ -30,27 +30,30 @@ shared_examples_for "files selenium shared" do path = "/dashboard/files" end context_code = context.asset_string.capitalize - resp, body = SSLCommon.get "#{app_host}#{path}", + resp = SSLCommon.get "#{app_host}#{path}", "Cookie" => @cookie + body = resp.body resp.code.should == "200" body.should =~ /
([^<]*)<\/div>/ authenticity_token = $1 - resp, body = SSLCommon.post_form("#{app_host}/files/pending", { + resp= SSLCommon.post_form("#{app_host}/files/pending", { "attachment[folder_id]" => context.folders.active.first.id, "attachment[filename]" => name, "attachment[context_code]" => context_code, "authenticity_token" => authenticity_token, "no_redirect" => true}, {"Cookie" => @cookie}) + body = resp.body resp.code.should == "200" data = json_parse(body) data["upload_url"] = data["proxied_upload_url"] || data["upload_url"] data["upload_url"] = "#{app_host}#{data["upload_url"]}" if data["upload_url"] =~ /^\// data["success_url"] = "#{app_host}#{data["success_url"]}" if data["success_url"] =~ /^\// data["upload_params"]["file"] = fixture - resp, body = SSLCommon.post_multipart_form(data["upload_url"], data["upload_params"], {"Cookie" => @cookie}, ["bucket", "key", "acl"]) + resp = SSLCommon.post_multipart_form(data["upload_url"], data["upload_params"], {"Cookie" => @cookie}, ["bucket", "key", "acl"]) + body = resp.body resp.code.should =~ /^20/ if body =~ // - resp, body = SSLCommon.get data["success_url"] + resp = SSLCommon.get data["success_url"] resp.code.should == "200" end end